Refactor bot system with simplified template and multi-bot support #4
Refactor bot system with simplified template and multi-bot support #4
Conversation
- Adopt clean listener pattern from bolt-ts-starter template - Add Assistant API support for DM/thread handling - Create ResponseHandler interface for customizable responses - Hook up AI SDK when AI provider is selected - Remove complex directories (db, memory, preferences, agents) - Organize handlers by type: assistant, events, messages - Process responses asynchronously for 3-second ack requirement
- Add waitForWebSocketConnection helper to verify emulator connection - Add IPC proxy for bot config fetch to bypass Electron CSP - Fix DynamicSettings to properly restore values on reload - Add user-friendly error messages for invalid API keys - Extract AI streaming logic to separate ai/ folder in template - Make ai/ folder conditional based on template options - Update default entry point to src/app.ts - Add test script and fix template test file
- Add targetBotId parameter to dispatchAppMentionEvent and dispatchEvent - Route app_mention events only to the specifically mentioned bot - Fix Settings.svelte to pass botId to DynamicSettings - Change DynamicSettings to use reactive instead of onMount so it waits for botId before fetching config in Electron mode
- Dynamically assign ports to each bot instead of using fixed port - Track config port per bot for IPC config fetching - Update bot:fetchConfig IPC to accept botId parameter - Update bots.yaml with simple and my-bot test instances - Clean up unused types and simplify state management
- Reduce heartbeat interval from 30s to 15s to stay within Slack SDK's timeout - Add ping handler to respond to client pings with pong - Add immediate ping on connection with logging - Fix package.json filter references to use correct package name
Show 🤔 reaction when AI starts processing and ✅ when complete. Reactions are added to DM handler, app_mention handler, and assistant message handler. Non-AI commands (like ping) skip reactions.
- Move SlackFile definition before its usage in SlackMessage - Add files array to SlackMessage for shortcut payload support - Update comments for clarity - Reorder types for better logical grouping
📝 WalkthroughWalkthroughReplaces YAML bot discovery with a compiled JSON manifest and manifest-driven bot startup; moves bot config retrieval to Electron IPC; adds per-bot ports, model-tier fetching and API-key validation in Electron; updates UI for bot-specific inherited settings; large refactor of create‑bot Slack template, emulator/server dispatch, and memory/DB surfaces. Changes
Sequence Diagram(s)sequenceDiagram
participant Electron as Electron main
participant Bot as Compiled Bot process
participant Emulator as Simulator server
participant Renderer as UI (renderer)
Electron->>Electron: readBotsManifest()
Electron->>Bot: spawn startBotProcess(botConfig, settingsEnv, botName, botPort)
Bot->>Bot: start local config server (bind port 0)
Bot->>Emulator: POST /config/register (configPort, token)
Emulator-->>Bot: registration response + simulator settings
Bot->>Bot: apply simulator settings to process.env
Renderer->>Electron: invoke 'bot:fetchConfig'(botId)
Electron->>Emulator: GET bot config (token-derived)
Emulator-->>Electron: bot config
Electron-->>Renderer: return bot config
Renderer->>Renderer: merge inherited global settings + bot overrides
sequenceDiagram
participant UI as Settings UI
participant Electron as Electron IPC
participant Provider as External AI Provider
participant Cache as Model Cache
UI->>Electron: invoke 'models:validateKey'(provider, apiKey)
Electron->>Provider: validate key & request model list
Provider-->>Electron: models / status
Electron->>Cache: cache tiers (ttl 30min)
Electron-->>UI: return validation result + tiers
UI->>UI: update model_select options and status indicators
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2026-01-16T23:30:02.205ZApplied to files:
🧬 Code graph analysis (1)packages/create-bot/templates/slack-bot/src/memory/index.ts (1)
🔇 Additional comments (8)
✏️ Tip: You can disable this entire section by setting Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ui/src/lib/backend-state.svelte.ts (1)
194-202: Guard against save failure leaving UI stuck disabled.
Ifapi.saveSettingsthrows,backendReadyremainsfalseandbackendErrorstaysnull, which can permanently disable input with no user feedback. Consider restoring the prior state and surfacing the error when the IPC call fails.💡 Suggested fix
async function saveSettings(newSettings: Record<string, unknown>) { const api = getElectronAPI() if (api) { // Electron mode: save via IPC // Set backendReady to false BEFORE the IPC call to avoid race condition // (backend:ready event might fire during the saveSettings call) - backendReady = false - await api.saveSettings(newSettings) + const prevReady = backendReady + backendReady = false + try { + await api.saveSettings(newSettings) + } catch (err) { + backendReady = prevReady + backendError = err instanceof Error ? err.message : String(err) + throw err + } } else { // Web mode: save to localStorage saveSettingsToStorage(newSettings) }
🤖 Fix all issues with AI agents
In `@apps/electron/electron.js`:
- Around line 188-203: The current logic only ensures botPort is free (via
findAvailablePort) but the config server binds botPort+1, so you must ensure
both ports are free before selecting them: add a helper isPortFree(port) that
attempts to bind to a specific port and returns true/false, then modify the port
selection flow that uses findAvailablePort()/botPort to loop or retry until both
isPortFree(botPort) and isPortFree(botPort + 1) are true (or pick a new botPort
if not), and use these checked values for botPort and configPort; update all
analogous usages (the other selection blocks around lines referenced) to use the
same isPortFree check before committing ports.
In `@apps/ui/src/components/DynamicSettings.svelte`:
- Around line 83-170: The async IIFE that calls fetchBotConfig lacks error
handling so a rejection leaves loading true; wrap the await
fetchBotConfig(botId) in a try/catch (or wrap the whole IIFE body) and on error
set a safe fallback (e.g., config = null or the simulator-only config), set
loading = false, and optionally set an error flag or log the error so the
existing "!config" error UI can render; update references to
formData/collapsedGroups only when config is valid to avoid runtime errors.
In `@apps/ui/src/lib/config-client.ts`:
- Around line 77-92: In fetchBotConfig, when isElectron is true and
getElectronAPI() returns null the code currently falls through to the web fetch
path; add an early return when api is falsy (e.g., log a warning and return
null) right after const api = getElectronAPI() so the function does not continue
to the non-Electron path; also improve the catch in fetchBotConfig to optionally
log the caught error from api.fetchBotConfig for visibility. Reference:
fetchBotConfig, isElectron, getElectronAPI, and api.fetchBotConfig.
In `@packages/create-bot/templates/slack-bot/src/ai/stream.ts.tmpl`:
- Around line 39-55: The OpenAI key validation should only run when the model is
an OpenAI model: before calling the /v1/models test, check the LanguageModelV1
instance's provider property (e.g., model.provider === 'openai') and only
perform the fetch when true; keep the existing behavior setting streamError and
using chunks, but skip the OpenAI API call for non-OpenAI providers (use
model.modelId if you need additional checks), so you don't surface a misleading
"Invalid API key" for other providers.
In `@packages/create-bot/templates/slack-bot/src/listeners/assistant/message.ts`:
- Around line 86-118: The streamer may remain open if
responseHandler.generateResponse throws inside the for-await loop in the
userMessage handler; ensure streamer.stop() is always called by moving cleanup
into a finally block that runs after the try/catch: wrap the
generateResponse/append loop and the reaction logic inside the try, keep the
catch to log and send the error reply, and in a finally block check that
streamer exists and call await streamer.stop() (and optionally swallow errors
from stop); reference responseHandler.generateResponse, streamer.append,
streamer.stop, and the userMessage handler when making this change.
In `@packages/create-bot/templates/slack-bot/src/listeners/events/app-mention.ts`:
- Around line 17-21: The current construction of RegExp using
slackConfig.app.name and slackConfig.app.id can misbehave if those values
contain regex metacharacters; update the code that builds the two new
RegExp(...) calls to escape regex-special characters first (e.g., implement or
import an escapeRegExp helper and call it for slackConfig.app.name and
slackConfig.app.id ?? ''), then create the RegExp from the escaped string with
the same 'gi' flags and use those to replace occurrences in the text variable;
ensure you still handle empty/undefined id gracefully by escaping an empty
string.
In `@packages/create-bot/templates/slack-bot/src/listeners/messages/index.ts`:
- Around line 43-45: The fire-and-forget invocation processMessage(client, say,
text, messageEvent, isThreadReply, threadTs) should append a .catch handler to
avoid unhandled promise rejections; update the call site to call
processMessage(...).catch(err => { /* log the error */ }) and surface the error
via the existing logger (or console.error) so any synchronous throw or rejection
outside processMessage's internal try block is caught and recorded.
In `@packages/create-bot/templates/slack-bot/src/settings.ts.tmpl`:
- Around line 33-35: The provider model defaults in the Slack template need
updating to avoid deprecations: in templates/slack-bot/src/settings.ts.tmpl
update the AI provider-specific default model values (where AI_PROVIDER and the
provider model defaults are defined) to pin OpenAI to a stable snapshot like
"gpt-4o-2024-11-20" instead of a floating "gpt-4o", replace Google’s deprecated
"gemini-2.0-flash" with "gemini-2.5-flash", and ensure Anthropic uses
"claude-sonnet-4-20250514"; change the enum/default strings and any
corresponding model-constant defaults to these exact values.
In `@packages/slack/src/server/socket-mode.ts`:
- Around line 211-213: The current socketModeLogger.debug calls log the full
incoming "data" object (e.g., the socketModeLogger.debug({ data }, 'Received
WebSocket message') call), which exposes user IDs and message text; update these
to log a redacted metadata object instead (for example include booleans like
hasText, textLength, event type, and a non-reversible user id hash) and remove
raw message text/user fields. Locate the two places where full "data" is logged
(the instance with socketModeLogger.debug({ data }, 'Received WebSocket
message') and the similar calls around lines 463-466) and replace them with a
call that builds and logs only safe metadata (compute a SHA-256 or other one-way
hash for user IDs or call an existing redaction utility if available) while
preserving the original log message string.
🧹 Nitpick comments (12)
packages/slack/src/server/types.ts (1)
35-50: Consider consolidatingSlackFiledefinitions.This
SlackFileinterface is a superset of the one inapps/ui/src/lib/types.ts. Having two separate definitions could lead to type drift over time.If the UI package imports from this server package, consider exporting a single source of truth. Otherwise, this duplication is acceptable if the separation is intentional (e.g., different package boundaries).
apps/electron/scripts/compile-bots.ts (1)
76-78: Consider a fallback tosrc/index.tsfor older bots.This keeps backward compatibility if a bot hasn’t migrated to
src/app.tsyet.♻️ Proposed compatibility fallback
- const entry = bot.entry ?? 'src/app.ts' - const sourcePath = path.resolve(ROOT_DIR, bot.source) - const entryPath = path.join(sourcePath, entry) + let entry = bot.entry ?? 'src/app.ts' + const sourcePath = path.resolve(ROOT_DIR, bot.source) + let entryPath = path.join(sourcePath, entry) + if (!bot.entry && !fs.existsSync(entryPath)) { + entry = 'src/index.ts' + entryPath = path.join(sourcePath, entry) + console.warn(` Default entry missing; falling back to ${entry}`) + }packages/create-bot/templates/slack-bot/src/app.ts.tmpl (2)
16-37: Add a per-request timeout to keep the poll deadline meaningful.
A stalled fetch can block the loop beyond the intended timeout; a short per‑request timeout keeps start‑up predictable.🔧 Suggested change
- const response = await fetch(`${baseUrl}/health`) + const response = await fetch(`${baseUrl}/health`, { + signal: AbortSignal.timeout(1000), + })
69-109: Guard simulator registration with a timeout (and tolerate non‑JSON errors).
A hung request will block retries; a timeout plus a safe JSON parse keeps the retry loop effective.🔧 Suggested change
- const response = await fetch(`${apiUrl}/config/register`, { + const response = await fetch(`${apiUrl}/config/register`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(slackConfig), + signal: AbortSignal.timeout(5000), }) - const data = (await response.json()) as { error?: string; message?: string } + const data = (await response.json().catch(() => ({}))) as { + error?: string + message?: string + }packages/create-bot/templates/slack-bot/src/ai/stream.ts.tmpl (1)
27-83: Consider yielding chunks as they arrive to preserve real streaming.
Buffering all chunks negates streaming and grows memory for long replies; if real‑time UX is desired, yield on each chunk and track ahasChunksflag.🔧 Suggested change
- const chunks: string[] = [] + let hasChunks = false let streamError: Error | null = null try { for await (const chunk of result.textStream) { - chunks.push(chunk) + hasChunks = true + yield chunk } } catch (error) { streamError = error as Error } - if (chunks.length === 0 && !streamError) { + if (!hasChunks && !streamError) { // Validate API key with a quick test call try { const testResponse = await fetch('https://api.openai.com/v1/models', { headers: { Authorization: `Bearer ${process.env.OPENAI_API_KEY}` }, signal: AbortSignal.timeout(5000), }) @@ - // Yield all collected chunks - for (const chunk of chunks) { - yield chunk - }apps/electron/electron.js (1)
862-897: Add a timeout to IPC config fetches to avoid hanging the renderer.
If the local fetch stalls, retries never happen; a short timeout keeps the IPC call responsive.🔧 Suggested change
- const response = await fetch(`http://127.0.0.1:${configPort}/config`) + const response = await fetch(`http://127.0.0.1:${configPort}/config`, { + signal: AbortSignal.timeout(1000), + })packages/create-bot/templates/slack-bot/src/response-handler.ts.tmpl (2)
42-45: Make the ping command tolerant of leading/trailing whitespace.This avoids missing common cases like
" ping ".🔧 Suggested tweak
- if (message.toLowerCase() === 'ping') { + if (message.trim().toLowerCase() === 'ping') { yield 'pong' return }
33-65: Avoid relying onthisinside the generator.If
generateResponseis passed around or destructured,thiscan beundefined, which risks losing the system prompt. Capture it in a local constant instead.♻️ Proposed change
-export const responseHandler: ResponseHandler = { - systemPrompt: '{{`#if` isAi}}You are a helpful assistant. Be concise and friendly.{{else}}You are a helpful assistant.{{/if}}', +const systemPrompt = + '{{`#if` isAi}}You are a helpful assistant. Be concise and friendly.{{else}}You are a helpful assistant.{{/if}}' + +export const responseHandler: ResponseHandler = { + systemPrompt, @@ - yield* streamAIResponse({ + yield* streamAIResponse({ model: getModel(), - system: this.systemPrompt, + system: systemPrompt, messages, })Run this to see whether
generateResponseis ever passed unbound:#!/bin/bash rg -nP "responseHandler\.generateResponse\b" -g'!**/*.test.*' rg -nP "const\s*\{\s*generateResponse\b" -g'!**/*.test.*'packages/create-bot/templates/slack-bot/src/listeners/assistant/thread-started.ts (1)
42-44: Consider whether silent error handling is appropriate.The error is logged but the user receives no feedback if the thread setup fails. Since
say()already executed before the error-prone operations, this is acceptable, but consider whether partial failures (e.g.,setSuggestedPromptsfailing) warrant user notification.apps/ui/src/lib/config-client.ts (1)
94-103: Inconsistent error handling between Electron and web paths.The Electron path logs a warning when
botIdis missing (line 81), but the web path silently returnsnullon fetch failures. Consider whether logging should be consistent for debugging purposes.apps/ui/src/components/Sidebar.svelte (1)
103-113: Inconsistent callback invocation between channel and bot rows.The channel row calls
handleSelectwhich triggers bothswitchChannelandonChannelSelect. The bot row only callsswitchChanneldirectly, skipping theonChannelSelectcallback. If any parent component relies ononChannelSelectfor tracking or side effects, this behaviour difference could cause subtle bugs.If intentional, consider adding a comment explaining why bot rows don't trigger
onChannelSelect.apps/ui/src/components/DynamicSettings.svelte (1)
137-144: Consider extractingisGlobalFieldlogic to avoid duplication.The same field scope determination logic exists here and in
getFieldsForGroup(lines 186-190). Extracting this to a helper function would improve maintainability.Proposed helper
function isGlobalField(key: string, schema: SettingSchema): boolean { return ( key === 'ai_provider' || key.endsWith('_api_key') || schema.group === 'advanced' ) }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (63)
apps/electron/bots.yamlapps/electron/electron.jsapps/electron/package.jsonapps/electron/scripts/compile-bots.tsapps/electron/src/preload.tsapps/ui/src/components/AppSettings.svelteapps/ui/src/components/DynamicSettings.svelteapps/ui/src/components/Settings.svelteapps/ui/src/components/Sidebar.svelteapps/ui/src/lib/backend-state.svelte.tsapps/ui/src/lib/config-client.tsapps/ui/src/lib/electron-api.tsapps/ui/src/lib/state.svelte.tsapps/ui/src/lib/types.tsbotarium.code-workspacebunfig.tomlpackage.jsonpackages/create-bot/src/scaffold.tspackages/create-bot/templates/slack-bot/bunfig.tomlpackages/create-bot/templates/slack-bot/config.yaml.tmplpackages/create-bot/templates/slack-bot/drizzle.config.ts.tmplpackages/create-bot/templates/slack-bot/package.json.tmplpackages/create-bot/templates/slack-bot/src/ai/agents/base.tspackages/create-bot/templates/slack-bot/src/ai/agents/general.tspackages/create-bot/templates/slack-bot/src/ai/agents/index.tspackages/create-bot/templates/slack-bot/src/ai/chat.tspackages/create-bot/templates/slack-bot/src/ai/stream.ts.tmplpackages/create-bot/templates/slack-bot/src/ai/tools/index.tspackages/create-bot/templates/slack-bot/src/ai/tools/memory.tspackages/create-bot/templates/slack-bot/src/app.ts.tmplpackages/create-bot/templates/slack-bot/src/config/http-server.ts.tmplpackages/create-bot/templates/slack-bot/src/config/loader.ts.tmplpackages/create-bot/templates/slack-bot/src/db/adapters/base.tspackages/create-bot/templates/slack-bot/src/db/adapters/postgres.tspackages/create-bot/templates/slack-bot/src/db/adapters/sqlite.tspackages/create-bot/templates/slack-bot/src/db/index.ts.tmplpackages/create-bot/templates/slack-bot/src/db/repository.tspackages/create-bot/templates/slack-bot/src/db/schema.tspackages/create-bot/templates/slack-bot/src/db/types.tspackages/create-bot/templates/slack-bot/src/index.ts.tmplpackages/create-bot/templates/slack-bot/src/listeners/assistant/context-changed.tspackages/create-bot/templates/slack-bot/src/listeners/assistant/index.tspackages/create-bot/templates/slack-bot/src/listeners/assistant/message.tspackages/create-bot/templates/slack-bot/src/listeners/assistant/thread-started.tspackages/create-bot/templates/slack-bot/src/listeners/events/app-mention.tspackages/create-bot/templates/slack-bot/src/listeners/events/index.tspackages/create-bot/templates/slack-bot/src/listeners/index.tspackages/create-bot/templates/slack-bot/src/listeners/messages/index.tspackages/create-bot/templates/slack-bot/src/memory/store.tspackages/create-bot/templates/slack-bot/src/preferences/index.tspackages/create-bot/templates/slack-bot/src/response-handler.test.ts.tmplpackages/create-bot/templates/slack-bot/src/response-handler.ts.tmplpackages/create-bot/templates/slack-bot/src/settings.ts.tmplpackages/create-bot/templates/slack-bot/src/slack/app.tspackages/create-bot/templates/slack-bot/src/slack/handlers.ts.tmplpackages/create-bot/templates/slack-bot/src/slack/thread-tracker.tspackages/create-bot/templates/slack-bot/src/utils/logger.tspackages/create-bot/templates/slack-bot/test/setup.tspackages/slack/src/server/index.tspackages/slack/src/server/socket-mode.tspackages/slack/src/server/state.tspackages/slack/src/server/types.tspackages/slack/src/server/web-api.ts
💤 Files with no reviewable changes (22)
- packages/create-bot/templates/slack-bot/src/db/types.ts
- packages/create-bot/templates/slack-bot/src/config/loader.ts.tmpl
- packages/create-bot/templates/slack-bot/src/ai/agents/index.ts
- packages/create-bot/templates/slack-bot/src/ai/tools/memory.ts
- packages/create-bot/templates/slack-bot/src/db/schema.ts
- packages/create-bot/templates/slack-bot/src/ai/agents/general.ts
- packages/create-bot/templates/slack-bot/src/slack/app.ts
- packages/create-bot/templates/slack-bot/src/index.ts.tmpl
- packages/create-bot/templates/slack-bot/src/db/adapters/sqlite.ts
- packages/create-bot/templates/slack-bot/src/ai/agents/base.ts
- packages/create-bot/templates/slack-bot/src/ai/tools/index.ts
- packages/create-bot/templates/slack-bot/src/db/adapters/postgres.ts
- packages/create-bot/templates/slack-bot/src/db/adapters/base.ts
- packages/create-bot/templates/slack-bot/src/ai/chat.ts
- packages/create-bot/templates/slack-bot/src/preferences/index.ts
- packages/create-bot/templates/slack-bot/src/memory/store.ts
- packages/create-bot/templates/slack-bot/src/db/index.ts.tmpl
- packages/create-bot/templates/slack-bot/src/slack/handlers.ts.tmpl
- packages/create-bot/templates/slack-bot/drizzle.config.ts.tmpl
- packages/create-bot/templates/slack-bot/src/slack/thread-tracker.ts
- apps/ui/src/lib/types.ts
- packages/create-bot/templates/slack-bot/src/db/repository.ts
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2026-01-16T23:29:55.585Z
Learnt from: tyom
Repo: tyom/botarium PR: 2
File: packages/create-bot/templates/slack-bot/src/slack/handlers.ts.tmpl:99-110
Timestamp: 2026-01-16T23:29:55.585Z
Learning: In the Slack bot template's handler (`packages/create-bot/templates/slack-bot/src/slack/handlers.ts.tmpl`), the `hasBotParticipatedInThread` function is a fallback path that only runs after bot restarts. The in-memory `threadTracker` handles active threads. The function intentionally limits to 100 messages to avoid multiple API calls and rate limit pressure for the rare edge case of threads with 100+ messages, where the failure mode is minor (bot won't auto-respond until re-mentioned).
Applied to files:
packages/create-bot/templates/slack-bot/src/response-handler.test.ts.tmplpackages/create-bot/templates/slack-bot/src/listeners/assistant/context-changed.tspackages/create-bot/templates/slack-bot/src/listeners/assistant/thread-started.tspackages/create-bot/templates/slack-bot/src/listeners/events/app-mention.tspackages/create-bot/templates/slack-bot/src/app.ts.tmplpackages/create-bot/templates/slack-bot/src/listeners/assistant/message.tspackages/create-bot/templates/slack-bot/src/listeners/messages/index.tspackages/create-bot/templates/slack-bot/src/response-handler.ts.tmplapps/ui/src/lib/state.svelte.ts
📚 Learning: 2026-01-14T21:36:07.933Z
Learnt from: tyom
Repo: tyom/botarium PR: 1
File: apps/ui/src/components/MessagePanel.svelte:10-23
Timestamp: 2026-01-14T21:36:07.933Z
Learning: In the client-only Botarium UI (Vite + Svelte), do not add or rely on SSR-specific guards (e.g., checks for sessionStorage) in components. Since the app runs only in Electron or the browser, code that accesses browser APIs will execute in a non-SSR environment, so SSR-unsafe checks are unnecessary and can be removed for maintainability.
Applied to files:
apps/ui/src/components/Settings.svelteapps/ui/src/components/Sidebar.svelteapps/ui/src/components/DynamicSettings.svelteapps/ui/src/components/AppSettings.svelte
📚 Learning: 2026-01-14T18:55:11.911Z
Learnt from: tyom
Repo: tyom/botarium PR: 1
File: apps/ui/src/index.html:6-9
Timestamp: 2026-01-14T18:55:11.911Z
Learning: The Botarium application is designed exclusively for Electron or localhost deployment with no plans for public/production deployment, so security configurations like Content-Security-Policy can be optimized for local development rather than production hardening.
Applied to files:
botarium.code-workspace
📚 Learning: 2026-01-16T19:57:15.905Z
Learnt from: tyom
Repo: tyom/botarium PR: 2
File: packages/create-bot/templates/slack-bot/src/db/index.ts.tmpl:12-23
Timestamp: 2026-01-16T19:57:15.905Z
Learning: In the create-botarium CLI scaffolding (packages/create-bot), the db/ directory is skipped entirely when dbAdapter === 'none' (see scaffold.ts shouldSkipDirectory). The isDb flag is true only when dbAdapter !== 'none', and when isDb is true, exactly one of isSqlite or isPostgres will be true based on the dbAdapter value. Therefore, template files in db/ are never rendered when no database is selected, and always have exactly one database adapter flag set when they are rendered.
Applied to files:
packages/create-bot/src/scaffold.ts
🧬 Code graph analysis (12)
packages/create-bot/templates/slack-bot/src/utils/logger.ts (2)
apps/electron/electron.js (2)
settings(346-346)settings(959-959)apps/ui/src/lib/backend-state.svelte.ts (1)
settings(278-280)
packages/create-bot/templates/slack-bot/src/listeners/events/index.ts (3)
packages/create-bot/templates/slack-bot/src/listeners/messages/index.ts (1)
register(14-46)packages/create-bot/templates/slack-bot/src/listeners/assistant/index.ts (1)
register(13-15)packages/create-bot/templates/slack-bot/src/listeners/events/app-mention.ts (1)
appMention(10-33)
packages/create-bot/templates/slack-bot/src/listeners/assistant/context-changed.ts (1)
packages/create-bot/templates/slack-bot/src/utils/logger.ts (1)
slackLogger(27-27)
packages/create-bot/templates/slack-bot/src/listeners/assistant/thread-started.ts (1)
packages/create-bot/templates/slack-bot/src/utils/logger.ts (1)
slackLogger(27-27)
packages/create-bot/templates/slack-bot/src/listeners/events/app-mention.ts (1)
packages/create-bot/templates/slack-bot/src/utils/logger.ts (1)
slackLogger(27-27)
packages/slack/src/server/web-api.ts (1)
packages/slack/src/lib/logger.ts (1)
webApiLogger(34-34)
packages/create-bot/templates/slack-bot/src/listeners/assistant/message.ts (2)
packages/slack/src/server/index.ts (1)
message(387-389)packages/create-bot/templates/slack-bot/src/utils/logger.ts (1)
slackLogger(27-27)
apps/ui/src/lib/config-client.ts (1)
apps/ui/src/lib/electron-api.ts (2)
isElectron(37-38)getElectronAPI(44-49)
packages/create-bot/templates/slack-bot/src/listeners/messages/index.ts (4)
packages/create-bot/templates/slack-bot/src/listeners/assistant/index.ts (1)
register(13-15)packages/create-bot/templates/slack-bot/src/listeners/events/index.ts (1)
register(4-6)packages/create-bot/templates/slack-bot/src/utils/logger.ts (1)
slackLogger(27-27)apps/electron/electron.js (2)
response(778-778)response(798-798)
packages/slack/src/server/types.ts (1)
apps/ui/src/lib/types.ts (1)
SlackFile(5-14)
apps/ui/src/lib/state.svelte.ts (1)
apps/ui/src/lib/types.ts (1)
CHANNELS(32-35)
packages/slack/src/server/socket-mode.ts (2)
packages/slack/src/lib/logger.ts (1)
socketModeLogger(35-35)packages/slack/src/server/types.ts (1)
SlackEvent(224-232)
🪛 ast-grep (0.40.5)
packages/create-bot/templates/slack-bot/src/listeners/events/app-mention.ts
[warning] 18-18: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(@?${slackConfig.app.name}, 'gi')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 19-19: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(@?${slackConfig.app.id ?? ''}, 'gi')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (59)
packages/slack/src/server/types.ts (5)
1-7: LGTM!Clear documentation updates that accurately describe the file's purpose as emulator-specific types.
52-66: LGTM!The addition of
filesarray alongside the existingfilefield mirrors the real Slack API structure, where messages can have both a primary file attachment and additional files with varying levels of metadata.
96-113: LGTM!The comment clearly documents the intentional deviation from
@slack/bolt'sSlashCommandtype. Omittingtokenfor the emulator is reasonable.
115-134: Verify thetype: 'shortcut'literal is intentional.The comment notes that the official
MessageShortcutfrom@slack/boltusestype: 'message_action', but this interface usestype: 'shortcut'. If bot handlers are written to match the official Slack API shape, they may not work correctly with this emulator payload.Consider aligning the type literal with the official API if compatibility is desired:
export interface MessageShortcutPayload { - type: 'shortcut' + type: 'message_action'Or, if
'shortcut'is intentional for emulator-specific routing, please add a comment explaining this choice.
312-321: LGTM!Clean and minimal interfaces for the reactions API endpoints.
packages/slack/src/server/web-api.ts (4)
134-141: LGTM!Whitelisting
/api/config/registerand/api/commands/registeris appropriate as these are internal simulator endpoints that don't require Slack-style token authentication. The comment clearly documents the intent.
253-259: LGTM!Good addition of debug logging for request tracing and error logging with diagnostic context (
hasChannel,hasTextflags) when validation fails. The debug level is appropriate for request body logging in an emulator context.
1063-1066: LGTM!Useful info-level logging for tracing incoming user messages through the simulator. Logging the key parameters (text, channel, user, thread_ts) aids debugging of message flows.
1119-1130: Method signature verification requiredThe multi-bot routing approach is conceptually sound—Slack's app_mention events don't include bot.id natively, so explicitly passing the bot ID ensures the event routes to the correct bot. However, the
dispatchAppMentionEventmethod signature must be verified in the codebase to confirm it accepts thebot.idparameter as shown in the call.packages/create-bot/templates/slack-bot/package.json.tmpl (2)
8-12: Verify the template entrypoint exists atsrc/app.ts.Lines 8–10 now run
src/app.ts. Please confirm the template actually includessrc/app.ts(and doesn't rely onsrc/index.ts), otherwise new bots will fail to start.
20-35: Confirm whether these pinned versions are intentional or require updating to current recommended versions.The template pins AI SDK v4.3.16 and provider packages v1.x (openai v1.3.22, anthropic v1.2.12, google v1.2.20). Current recommended versions are significantly newer: AI SDK v6, and all providers v2.x. Between these versions, there are substantial API breaking changes (useChat/onFinish signatures, streaming protocol, tools definitions, Agent-first patterns in v6). If this version lock is unintentional, the template will break at runtime unless code uses the deprecated v4 APIs. If intentional, document this constraint.
packages/slack/src/server/index.ts (1)
393-396: Ping handler keeps Socket Mode connections responsive.Nice addition. Please confirm that the Bun WebSocket server emits
pingcallbacks for client pings and thatws.pong(data)is supported in the Bun version you ship, so keep‑alive behaviour stays consistent.packages/slack/src/server/socket-mode.ts (2)
34-34: Heartbeat tightening and immediate ping are sensible.This should reduce stale sockets. Please verify the 15s cadence and payload pings are compatible with the Slack SDK/Bun WebSocket client behaviour in your runtime, especially the immediate ping on open.
Also applies to: 93-96, 187-199
304-412: Targeted dispatch routing looks clear and avoids cross‑bot fan‑out.The DM/targetBotId selection and scoped send loop align with multi‑bot support; no concerns.
Also applies to: 487-500
packages/create-bot/templates/slack-bot/test/setup.ts (1)
1-2: LGTM: test env bootstrapping is clear.This sets the Slack API base URL before modules load, which stabilises tests.
packages/create-bot/templates/slack-bot/bunfig.toml (1)
1-2: LGTM: test preload wiring.Preloading the setup file ensures the env is set consistently for tests.
packages/create-bot/src/scaffold.ts (1)
112-114: LGTM: comment aligns with logic.The updated wording better reflects the AI selection behaviour.
botarium.code-workspace (1)
54-63: Verify thedev:electronscript exists inapps/electron/package.json.With the cwd change to
apps/electron, the task resolves scripts from that directory's package.json. Ifdev:electronis defined only at the repository root, this task will fail. Confirm the script exists at the correct location before merging.apps/electron/bots.yaml (1)
14-18: Please confirm these bot paths exist and are intended to ship.If
../../simpleor../../my-botare missing in certain environments, bundling will silently skip them.If helpful, you can quickly verify with:
#!/bin/bash # Verify bot directories and default entry points exist relative to repo root. set -euo pipefail test -d simple test -d my-bot test -f simple/src/app.ts test -f my-bot/src/app.tspackages/create-bot/templates/slack-bot/src/utils/logger.ts (1)
4-10: LGTM! Clear conditional logging configuration.The logic correctly switches to JSON output for simulator mode (for Electron parsing) whilst using pretty output only in interactive terminal sessions during local development.
apps/ui/src/lib/state.svelte.ts (1)
187-202: LGTM! Clean separation of DM and channel validation.The updated logic correctly handles:
- Early return for empty channel IDs
- Dynamic DM channels (
D_*) validated at runtime when navigating- Static channels validated against the
CHANNELSarrayThis aligns well with the multi-bot support where DM channels are dynamically created per bot.
packages/create-bot/templates/slack-bot/src/config/http-server.ts.tmpl (2)
47-47: Intentional simplification of model_tiers.The
model_tiersnow returns an empty object, which aligns with the PR's broader simplification of model/provider configuration. Consumers should be aware that this field will no longer contain dynamic data.
61-61: Good addition for Electron compatibility.Explicitly binding to
127.0.0.1ensures consistent IPv4 behaviour across platforms, avoiding potential issues wherelocalhostmight resolve to IPv6 on some systems.packages/slack/src/server/state.ts (2)
317-324: LGTM! Consistent DM detection with dynamic channel pattern.The prefix-based check for
D_channels correctly handles the dynamic DM pattern introduced for multi-bot support, whilst maintaining backward compatibility with the channel map lookup.
613-628: LGTM! Sensible bot ID assignment strategy.Using
app.idfrom the config when available provides consistent bot identification across the system, with the UUID fallback ensuring robustness for bots without explicit IDs.packages/create-bot/templates/slack-bot/src/listeners/assistant/context-changed.ts (1)
1-12: LGTM! Clean assistant context middleware.The implementation correctly handles the context change event with appropriate error handling. Logging and swallowing errors is suitable here as context-saving failures shouldn't interrupt the user flow.
packages/create-bot/templates/slack-bot/src/listeners/events/app-mention.ts (3)
31-33: Intentional fire-and-forget pattern for Slack's 3-second ack requirement.The unawaited call to
processMentionis correct for satisfying Slack's acknowledgement deadline. Error handling withinprocessMentionensures failures are logged and the user is notified.
45-79: LGTM! Solid reaction-based UX feedback with robust error handling.The implementation provides good user feedback (thinking → checkmark reactions) and handles errors gracefully with cleanup attempts and user notification.
53-59: Consider populatingteamIdif required by response generation.
teamIdis set to an empty string. IfresponseHandler.generateResponseor downstream logic requires team context, you may need to extract it from the event or client.packages/create-bot/templates/slack-bot/src/app.ts.tmpl (2)
8-67: Simulator vs live app wiring is clear.
113-180: Start‑up orchestration and shutdown hooks look robust.apps/ui/src/components/AppSettings.svelte (2)
18-26: Props destructuring is cleaner with direct appId usage.
95-101: DynamicSettings now receives app overrides and bot context.packages/create-bot/templates/slack-bot/src/ai/stream.ts.tmpl (1)
8-12: StreamOptions is a tidy contract for callers.packages/create-bot/templates/slack-bot/src/listeners/assistant/message.ts (2)
10-15: Non‑AI command filtering is simple and readable.
78-84: The suggested fix contradicts Slack's API requirements for channel/thread streaming.According to Slack's official documentation,
recipient_team_idandrecipient_user_idare required parameters when streaming to channels or threads (as opposed to direct messages). Since the code passes achannelparameter, these fields are API-required. Conditionally omitting them when undefined—as the suggested change proposes—would cause the exact API-side validation errors the review comment warns against.If
userIdorteamIdmay be undefined in this context, the proper fix is to either:
- Ensure these values are populated before reaching this code
- Use fallback values from the message context when they're unavailable
- Not send to a channel/thread when these identifiers cannot be determined
Removing required fields is not a valid solution.
Likely an incorrect or invalid review comment.
packages/create-bot/templates/slack-bot/config.yaml.tmpl (1)
31-103: Conditional AI settings and consistent key labelling look good.apps/electron/electron.js (3)
19-49: Port‑aware process tracking is a solid base for multi‑bot support.Also applies to: 759-762
291-321: Extra diagnostics and botPorts in status are helpful for troubleshooting.Also applies to: 342-384, 816-835, 846-855
1099-1114: The before‑quit guard prevents double clean‑up.packages/create-bot/templates/slack-bot/src/settings.ts.tmpl (1)
31-49: Conditional local‑mode defaults look consistent.The local‑mode defaults and required checks read cleanly and should keep simulator setup friction low.
packages/create-bot/templates/slack-bot/src/response-handler.test.ts.tmpl (1)
4-29: Solid minimal contract tests.Covers prompt presence, suggested prompts, and the ping‑pong path succinctly.
bunfig.toml (1)
1-2: Simple test config placeholder is fine.No concerns here.
packages/create-bot/templates/slack-bot/src/listeners/events/index.ts (1)
4-6: App‑mention wiring is straightforward.Please just sanity‑check the event name against your Slack Bolt version.
apps/ui/src/lib/electron-api.ts (1)
29-31: [rewritten comment]
[classification tag]apps/electron/src/preload.ts (1)
46-49: LGTM!The new
fetchBotConfigIPC method follows the established patterns in this preload script. ThePromise<unknown | null>return type is appropriate for the preload layer, allowing type narrowing to occur in the consuming code.packages/create-bot/templates/slack-bot/src/listeners/index.ts (1)
1-10: LGTM!Clean aggregation pattern for Slack Bolt listeners. The namespace imports provide clear grouping, and the single
registerListenersentry point simplifies app bootstrapping.apps/ui/src/components/Settings.svelte (2)
11-17: LGTM!The derived
firstBotIdcorrectly handles the case when no bots are connected by returningundefined. The comment clearly explains the rationale that any bot suffices for fetching the global settings schema.
131-136: VerifyDynamicSettingshandlesundefinedbotId gracefully.When no bots are connected,
firstBotIdwill beundefined. EnsureDynamicSettingshandles this state appropriately (e.g., shows a loading state or defers config fetching).packages/create-bot/templates/slack-bot/src/listeners/assistant/index.ts (1)
1-15: LGTM!Clean implementation following the Slack Bolt Assistant API pattern. Module-level instantiation is appropriate here as there's one assistant instance per bot, and the
registerfunction cleanly separates app wiring from assistant configuration.packages/create-bot/templates/slack-bot/src/listeners/assistant/thread-started.ts (1)
5-18: FixsaveThreadContext()call to pass the context parameter.The Slack Bolt JavaScript API signature for
saveThreadContextrequires acontextparameter:saveThreadContext(context). The function forwards this context object to the AssistantContextStore. Currently, the code callssaveThreadContext()with no arguments, which means the context will not be properly persisted. Pass thecontextobject extracted from the event:await saveThreadContext(context)⛔ Skipped due to learnings
Learnt from: tyom Repo: tyom/botarium PR: 2 File: packages/create-bot/templates/slack-bot/src/slack/handlers.ts.tmpl:99-110 Timestamp: 2026-01-16T23:29:55.585Z Learning: In the Slack bot template's handler (`packages/create-bot/templates/slack-bot/src/slack/handlers.ts.tmpl`), the `hasBotParticipatedInThread` function is a fallback path that only runs after bot restarts. The in-memory `threadTracker` handles active threads. The function intentionally limits to 100 messages to avoid multiple API calls and rate limit pressure for the rare edge case of threads with 100+ messages, where the failure mode is minor (bot won't auto-respond until re-mentioned).packages/create-bot/templates/slack-bot/src/listeners/messages/index.ts (4)
1-12: LGTM!Clean imports and the
isAIResponsehelper provides a clear mechanism to distinguish AI-triggering messages from simple commands.
96-108: LGTM!Good UX pattern: removing the thinking reaction and adding the checkmark provides clear visual feedback to users. The
.catch()handlers prevent reaction failures from blocking the main flow.
122-133: LGTM!Helpful user-facing error messages that distinguish between authentication issues and rate limits. This aligns well with the PR objective for user-friendly error messages for invalid API keys.
70-76: Consider populatingteamIdfrom the event.
teamIdis hardcoded to an empty string, butGenericMessageEventtypically includes ateamproperty. If downstream code relies onteamIdfor context or logging, this could cause issues.apps/ui/src/components/Sidebar.svelte (2)
67-93: LGTM!Clean separation of channel rendering with proper keyboard accessibility via
handleKeyDown.
129-139: LGTM!Good conditional rendering: settings button only appears when the bot is connected and the callback is provided. The
e.stopPropagation()correctly prevents the row click from firing.apps/ui/src/components/DynamicSettings.svelte (2)
48-69: LGTM!The reactive sync logic correctly handles initial values and re-syncs when they change, whilst preserving user modifications. The comparison with
prevValueensures user edits aren't overwritten.
161-166: LGTM!Collapsed state initialisation from group definitions is clean and integrates well with the existing
toggleGroupmechanism.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
packages/create-bot/templates/slack-bot/src/listeners/assistant/message.ts
Outdated
Show resolved
Hide resolved
packages/create-bot/templates/slack-bot/src/listeners/events/app-mention.ts
Outdated
Show resolved
Hide resolved
- Use unique tokens per bot (xoxb-{botId}) for proper identification
- Push global settings from Electron to emulator on startup
- Include simulator settings in bot registration response
- Add UI helpers to identify and display correct bot names
- Add AI provider settings to simulator schema for discovery mode
Bots now re-register with the simulator when reconnecting after a simulator restart. Also adds configPort to bot registration so Electron can fetch settings for externally-started bots.
Config servers now use port 0 to let the OS pick an available port, eliminating port conflicts when running multiple bots simultaneously. The actual port is sent during registration to the emulator.
Config servers now use random ports, so the hardcoded DEFAULT_CONFIG_PORT and web mode fallback are no longer useful. Simplified to Electron-only.
- Use JSON for bot configuration instead of YAML - compile-bots.ts generates manifest.json with resolved bot names - electron.js reads from manifest instead of parsing YAML - Consolidate build scripts (dev, build, package)
- Fix bot name mix-up: derive bot name from DM channel ID instead of
using global botName state (state.svelte.ts)
- Fix reaction code in templates: wrap thinking/checkmark reactions in
{{#if isAi}} conditionals so non-AI bots don't show them
- Fix settings dialog empty: query emulator for configPort first since
bots use random ports, not the calculated PORT+1 that Electron assumed
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/electron/electron.js (1)
656-668: Inconsistent log message referencesbots.yamlbut config is now JSON.Line 656 and 668 still reference
bots.yamlin log messages, but the configuration source has been migrated tobots.jsonandmanifest.json.🐛 Suggested fix
if (botConfigs.length > 0) { - botProcLogger.info(`Starting ${botConfigs.length} bot(s) from bots.yaml`) + botProcLogger.info(`Starting ${botConfigs.length} bot(s) from manifest`) for (const config of botConfigs) { // Find an available port for this bot const botPort = await findAvailablePort() ... } else { botProcLogger.info( - 'No bots configured in bots.yaml - running in discovery mode' + 'No bots configured in manifest - running in discovery mode' ) }packages/slack/src/server/web-api.ts (2)
155-176: Reject unknown bot tokens to prevent mis‑attribution.
At the moment anyxoxb-*token passes validation even if no bot exists, which can route requests to the global fallback and blur bot identity. Consider rejecting unknown bot IDs withinvalid_auth.🛠️ Suggested guard for unknown bot tokens
- const token = req.headers.get('Authorization')?.replace('Bearer ', '') ?? null + const token = req.headers.get('Authorization')?.replace('Bearer ', '') ?? null + const botId = this.extractBotIdFromToken(token) + const botInfo = botId ? this.state.getBotInfoById(botId) : undefined + + const isWhitelistedPath = + !path.startsWith('/api/') || + path === '/api/apps.connections.open' || + path === '/api/config/register' || + path === '/api/commands/register' - const isValidToken = - token?.startsWith('xoxb-') || - token?.startsWith('xoxp-') || - !path.startsWith('/api/') || - path === '/api/apps.connections.open' || - path === '/api/config/register' || - path === '/api/commands/register' + const isValidToken = + token?.startsWith('xoxb-') || + token?.startsWith('xoxp-') || + isWhitelistedPath if (!isValidToken && path.startsWith('/api/')) { return Response.json( { ok: false, error: 'invalid_auth' }, { status: 401, headers: corsHeaders() } ) } + + if (token?.startsWith('xoxb-') && !botInfo && !isWhitelistedPath) { + return Response.json( + { ok: false, error: 'invalid_auth' }, + { status: 401, headers: corsHeaders() } + ) + }
276-323: Allow block/attachment‑onlychat.postMessagecalls.Slack's Web API specifies that
textis optional whenblocksorattachmentsare provided, but at least one of the three must be present. The current validation rejects requests withblocksorattachmentswhentextis omitted, preventing bots from sending rich messages. Slack also recommends including a top-leveltextas a fallback for notifications and accessibility.Update the validation to accept requests with any combination of
text,blocks, orattachments, and use an empty string fallback whentextis absent:Suggested implementation
- const { channel, text, thread_ts } = body + const { channel, text, thread_ts, blocks, attachments } = body + const hasContent = + !!text || + (Array.isArray(blocks) && blocks.length > 0) || + (Array.isArray(attachments) && attachments.length > 0) - if (!channel || !text) { + if (!channel || !hasContent) { webApiLogger.error( - { channel, text, hasChannel: !!channel, hasText: !!text }, + { + channel, + hasChannel: !!channel, + hasText: !!text, + hasBlocks: Array.isArray(blocks) && blocks.length > 0, + hasAttachments: Array.isArray(attachments) && attachments.length > 0, + }, 'chat.postMessage missing required argument' ) return Response.json( { ok: false, error: 'missing_argument' }, { headers: corsHeaders() } ) } const botInfo = this.getBotInfoFromToken(token) + const messageText = text ?? '' const ts = this.state.generateTimestamp() const message: SlackMessage = { type: 'message', channel, user: botInfo.id, - text, + text: messageText, ts, thread_ts, } const response: ChatPostMessageResponse = { ok: true, channel, ts, message: { type: 'message', - text, + text: messageText, user: botInfo.id, ts, }, } - webApiLogger.debug(`chat.postMessage: ${text.substring(0, 50)}...`) + const preview = messageText ? `${messageText.substring(0, 50)}...` : '[no text]' + webApiLogger.debug(`chat.postMessage: ${preview}`)
🤖 Fix all issues with AI agents
In `@apps/electron/scripts/compile-bots.ts`:
- Around line 66-77: Wrap the JSON.parse call that assigns "config" from
"content" in a try-catch to handle malformed JSON: catch the parsing error, log
a clear message including CONFIG_PATH and the error details (e.g., using
console.error) and exit with a non-zero status instead of letting the exception
crash the script; ensure the existing variables ("content", "config" typed as
BotsConfig, and "botEntries") are preserved and only proceed if parsing
succeeds.
In `@packages/create-bot/templates/slack-bot/src/app.ts.tmpl`:
- Around line 82-90: The registerWithSimulator function currently builds a
registrationPayload using configServerPort even when it's undefined; update
registerWithSimulator to check the configServerPort (the value returned by
startConfigServer) and guard early: if configServerPort is falsy, skip or fail
the registration (e.g., return early or throw) so you don't call the simulator
with an undefined app.configPort; apply the same guard where the code later
registers (the other simulator registration block that uses slackConfig and
configServerPort) to ensure neither registration proceeds when startConfigServer
failed.
In `@packages/create-bot/templates/slack-bot/src/listeners/events/app-mention.ts`:
- Around line 33-35: The fire-and-forget call to processMention in
app-mention.ts needs a .catch() to avoid unhandled promise rejections; update
the call to append .catch(err => { /* handle/log error */ }) so any synchronous
throw or early rejection is caught — reference the processMention invocation and
ensure the catch logs or reports the error (e.g., using console.error or say)
while preserving the existing asynchronous ack behavior.
In `@packages/slack/src/server/state.ts`:
- Around line 300-313: getBotInfoById currently constructs the bot user id as
`U_${appConfig.app.id}` which yields `U_undefined` for legacy bots; update
getBotInfoById to detect a missing or falsy appConfig.app.id and fall back to
the provided botId (e.g., use `appConfig.app.id` if present else `botId`) so the
returned id is always unique and never `U_undefined`; keep name and display_name
as appConfig.app.name as before.
♻️ Duplicate comments (4)
apps/electron/electron.js (1)
187-202: TOCTOU race: port may be taken between discovery and bot startup.The port is released when
server.close()completes, but another process could claim it before the bot binds. This is inherent to ephemeral port discovery but worth noting. The bot should handleEADDRINUSEgracefully.Additionally, as noted in a previous review,
configPortis computed asbotPort + 1(line 552), but there's no guarantee that port is free.#!/bin/bash # Check if the bot's config server handles port binding failures gracefully rg -n "EADDRINUSE|address already in use" --type tspackages/create-bot/templates/slack-bot/src/listeners/events/app-mention.ts (1)
19-23: Escape regex special characters to prevent unexpected behaviour.The regex escaping issue flagged previously is still unaddressed. Config values like
slackConfig.app.namemay contain regex metacharacters (e.g., parentheses, brackets) that could cause syntax errors or unexpected matching.🛠️ Suggested fix
+// Escape regex special characters +function escapeRegex(str: string): string { + return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') +} + // Strip bot mention from message text const text = event.text .replace(/<@[A-Z0-9]+(\|[^>]*)?>/g, '') - .replace(new RegExp(`@?${slackConfig.app.name}`, 'gi'), '') - .replace(new RegExp(`@?${slackConfig.app.id ?? ''}`, 'gi'), '') + .replace(new RegExp(`@?${escapeRegex(slackConfig.app.name)}`, 'gi'), '') + .replace(new RegExp(`@?${escapeRegex(slackConfig.app.id ?? '')}`, 'gi'), '') .trim()packages/create-bot/templates/slack-bot/src/listeners/assistant/message.ts (1)
94-108: Ensure streamer is stopped on failure to avoid dangling streams.If
generateResponsethrows during the append loop, the stream is never closed and remains in an active state. This can leave the UI in a partial state and leak resources.🛠️ Suggested fix
// Generate and stream response + let streamStopped = false const streamer = client.chatStream({ channel, recipient_team_id: teamId, recipient_user_id: userId, thread_ts, }) - for await (const chunk of responseHandler.generateResponse( - message.text, - threadContext - )) { - await streamer.append({ markdown_text: chunk }) + try { + for await (const chunk of responseHandler.generateResponse( + message.text, + threadContext + )) { + await streamer.append({ markdown_text: chunk }) + } + await streamer.stop() + streamStopped = true + } finally { + if (!streamStopped) { + await streamer.stop().catch(() => {}) + } } - - await streamer.stop()packages/create-bot/templates/slack-bot/src/listeners/messages/index.ts (1)
47-49: Add.catch()to the fire-and-forget call to prevent unhandled promise rejections.The fire-and-forget invocation of
processMessageshould have a.catch()handler. Although the function has internal error handling, any synchronous throw or rejection outside the try block would be unhandled.🛠️ Suggested fix
// Process asynchronously to ack within 3 seconds - processMessage({{`#if` isAi}}client, {{/if}}say, text, messageEvent, isThreadReply, threadTs) + processMessage({{`#if` isAi}}client, {{/if}}say, text, messageEvent, isThreadReply, threadTs).catch(err => + slackLogger.error({ err }, 'Unhandled error in processMessage') + ) })
🧹 Nitpick comments (5)
apps/ui/src/lib/simulator-settings.ts (1)
19-22: Consider using a union type forai_provider.Using a string union type would provide compile-time safety and autocomplete support. This is optional but improves maintainability.
♻️ Suggested improvement
+export type AIProvider = 'openai' | 'anthropic' | 'google' + export interface SimulatorSettings { simulated_user_name: string emulator_port: number data_dir: string log_level: LogLevel - ai_provider: string + ai_provider: AIProvider openai_api_key?: string anthropic_api_key?: string google_api_key?: string }packages/slack/src/server/state.ts (1)
1019-1026: Avoid exposing mutable simulatorSettings references.Storing and returning the same object allows external mutation without logs. Consider shallow copies on set/get to keep state changes explicit.
♻️ Suggested change
setSimulatorSettings(settings: Record<string, unknown>): void { - this.simulatorSettings = settings + this.simulatorSettings = { ...settings } stateLogger.debug({ keys: Object.keys(settings) }, 'Simulator settings updated') } getSimulatorSettings(): Record<string, unknown> { - return this.simulatorSettings + return { ...this.simulatorSettings } }apps/electron/scripts/compile-bots.ts (1)
128-135: Consider excluding undefinedentryfields from manifest.When
bot.entryisundefined, the manifest will contain{ name: "...", entry: undefined }. While JSON.stringify omitsundefinedvalues, it may be clearer to explicitly filter:♻️ Optional refinement
const compiledBots = bots .filter((bot) => fs.existsSync(path.join(OUTPUT_DIR, bot.name))) - .map((bot) => ({ name: bot.name, entry: bot.entry })) + .map((bot) => { + const result: { name: string; entry?: string } = { name: bot.name } + if (bot.entry) result.entry = bot.entry + return result + })packages/slack/src/server/types.ts (2)
35-65: ReuseSlackFile-derived types for message attachments to avoid drift.The inline
{ mimetype, url_private }shape duplicatesSlackFile. Using aPick/Partialkeeps a single source of truth while preserving the lightweight payload.♻️ Suggested refactor
- files?: Array<{ - mimetype?: string - url_private?: string - }> + files?: Array< + Partial<Pick<SlackFile, 'mimetype' | 'url_private'>> + >
85-90: Update theconfigPortcomment to match dynamic allocation.The note “usually bot port + 1” is likely stale with random OS-assigned ports.
✏️ Suggested tweak
- configPort?: number // Port for bot's config HTTP server (usually bot port + 1) + configPort?: number // Port for bot's config HTTP server (assigned at runtime and reported during registration)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (22)
apps/electron/README.mdapps/electron/bots.jsonapps/electron/bots.yamlapps/electron/electron.jsapps/electron/package.jsonapps/electron/scripts/compile-bots.tsapps/ui/src/components/Message.svelteapps/ui/src/lib/config-client.tsapps/ui/src/lib/dispatcher.svelte.tsapps/ui/src/lib/simulator-settings.tsapps/ui/src/lib/state.svelte.tsbotarium.code-workspacepackage.jsonpackages/create-bot/templates/slack-bot/src/app.ts.tmplpackages/create-bot/templates/slack-bot/src/config/http-server.ts.tmplpackages/create-bot/templates/slack-bot/src/listeners/assistant/message.tspackages/create-bot/templates/slack-bot/src/listeners/events/app-mention.tspackages/create-bot/templates/slack-bot/src/listeners/messages/index.tspackages/slack/src/server/index.tspackages/slack/src/server/state.tspackages/slack/src/server/types.tspackages/slack/src/server/web-api.ts
💤 Files with no reviewable changes (1)
- apps/electron/bots.yaml
✅ Files skipped from review due to trivial changes (2)
- apps/electron/bots.json
- apps/electron/README.md
🚧 Files skipped from review as they are similar to previous changes (4)
- botarium.code-workspace
- packages/slack/src/server/index.ts
- apps/electron/package.json
- package.json
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: tyom
Repo: tyom/botarium PR: 1
File: apps/ui/src/index.html:6-9
Timestamp: 2026-01-14T18:55:11.911Z
Learning: The Botarium application is designed exclusively for Electron or localhost deployment with no plans for public/production deployment, so security configurations like Content-Security-Policy can be optimized for local development rather than production hardening.
📚 Learning: 2026-01-16T23:29:55.585Z
Learnt from: tyom
Repo: tyom/botarium PR: 2
File: packages/create-bot/templates/slack-bot/src/slack/handlers.ts.tmpl:99-110
Timestamp: 2026-01-16T23:29:55.585Z
Learning: In the Slack bot template's handler (`packages/create-bot/templates/slack-bot/src/slack/handlers.ts.tmpl`), the `hasBotParticipatedInThread` function is a fallback path that only runs after bot restarts. The in-memory `threadTracker` handles active threads. The function intentionally limits to 100 messages to avoid multiple API calls and rate limit pressure for the rare edge case of threads with 100+ messages, where the failure mode is minor (bot won't auto-respond until re-mentioned).
Applied to files:
packages/create-bot/templates/slack-bot/src/listeners/assistant/message.tsapps/ui/src/lib/state.svelte.tspackages/create-bot/templates/slack-bot/src/app.ts.tmplpackages/create-bot/templates/slack-bot/src/listeners/messages/index.tspackages/create-bot/templates/slack-bot/src/listeners/events/app-mention.ts
📚 Learning: 2026-01-14T21:36:07.933Z
Learnt from: tyom
Repo: tyom/botarium PR: 1
File: apps/ui/src/components/MessagePanel.svelte:10-23
Timestamp: 2026-01-14T21:36:07.933Z
Learning: In the client-only Botarium UI (Vite + Svelte), do not add or rely on SSR-specific guards (e.g., checks for sessionStorage) in components. Since the app runs only in Electron or the browser, code that accesses browser APIs will execute in a non-SSR environment, so SSR-unsafe checks are unnecessary and can be removed for maintainability.
Applied to files:
apps/ui/src/components/Message.svelte
📚 Learning: 2026-01-14T21:36:12.466Z
Learnt from: tyom
Repo: tyom/botarium PR: 1
File: apps/ui/src/components/MessagePanel.svelte:10-23
Timestamp: 2026-01-14T21:36:12.466Z
Learning: The Botarium UI is a client-only Vite + Svelte application with no server-side rendering; it runs exclusively in Electron or browser environments, so checks for SSR-unsafe browser APIs like sessionStorage are unnecessary.
Applied to files:
apps/electron/electron.js
🧬 Code graph analysis (9)
apps/ui/src/components/Message.svelte (2)
apps/ui/src/lib/backend-state.svelte.ts (1)
syncUserSettingsToState(16-18)apps/ui/src/lib/types.ts (1)
ConnectedBotInfo(239-246)
apps/ui/src/lib/dispatcher.svelte.ts (1)
apps/ui/src/lib/state.svelte.ts (1)
isBotUserId(489-504)
apps/electron/scripts/compile-bots.ts (1)
apps/electron/electron.js (2)
content(253-253)manifestPath(243-245)
apps/ui/src/lib/state.svelte.ts (2)
apps/ui/src/lib/types.ts (4)
CHANNELS(32-35)BOT_USER_ID(37-37)ConnectedBotInfo(238-245)Channel(26-30)packages/create-bot/templates/slack-bot/src/ai/chat.ts (1)
ChatContext(5-10)
packages/slack/src/server/types.ts (1)
apps/ui/src/lib/types.ts (2)
SlackFile(5-14)SlackAppConfig(57-65)
apps/ui/src/lib/simulator-settings.ts (3)
apps/ui/src/lib/settings-store.ts (1)
getApiKey(29-35)apps/ui/src/lib/backend-state.svelte.ts (1)
migrateSettings(49-96)apps/ui/src/lib/backend-state.test.ts (1)
globalSettings(36-52)
packages/slack/src/server/state.ts (3)
apps/electron/electron.js (2)
settings(304-304)settings(953-953)apps/ui/src/lib/backend-state.svelte.ts (1)
settings(278-280)packages/slack/src/lib/logger.ts (1)
stateLogger(33-33)
packages/slack/src/server/web-api.ts (2)
packages/slack/src/server/types.ts (2)
ChatPostMessageRequest(257-263)ReactionsRequest(313-317)packages/slack/src/lib/logger.ts (1)
webApiLogger(34-34)
apps/electron/electron.js (2)
apps/ui/src/lib/backend-state.svelte.ts (1)
settings(278-280)packages/slack/src/server/index.ts (1)
fetch(97-415)
🪛 ast-grep (0.40.5)
packages/create-bot/templates/slack-bot/src/listeners/events/app-mention.ts
[warning] 20-20: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(@?${slackConfig.app.name}, 'gi')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 21-21: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(@?${slackConfig.app.id ?? ''}, 'gi')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🪛 Biome (2.1.2)
packages/create-bot/templates/slack-bot/src/listeners/assistant/message.ts
[error] 2-2: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 2-2: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 4-4: Expected a statement but instead found 'else'.
Expected a statement here.
(parse)
[error] 9-9: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 9-10: Illegal use of an import declaration not at the top level
move this declaration to the top level
(parse)
[error] 10-11: Illegal use of an import declaration not at the top level
move this declaration to the top level
(parse)
[error] 13-13: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 13-13: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 21-21: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 41-41: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 41-41: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 43-43: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 45-45: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 45-45: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 47-47: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 56-56: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 56-56: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 66-66: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 76-76: Expected a property, a shorthand property, a getter, a setter, or a method but instead found '{#if isAi'.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
[error] 77-78: Expected a property, a shorthand property, a getter, a setter, or a method but instead found '{'.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
[error] 78-78: Expected a property, a shorthand property, a getter, a setter, or a method but instead found '{else'.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
[error] 78-78: expected , but instead found }
Remove }
(parse)
[error] 79-79: expected , but instead found thread
Remove thread
(parse)
[error] 79-80: Expected a property, a shorthand property, a getter, a setter, or a method but instead found '{'.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
[error] 80-80: Expected a property, a shorthand property, a getter, a setter, or a method but instead found '{/if'.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
[error] 80-80: expected , but instead found }
Remove }
(parse)
[error] 81-81: expected , but instead found role
Remove role
(parse)
[error] 81-81: expected , but instead found :
Remove :
(parse)
[error] 81-81: expected , but instead found (
Remove (
(parse)
[error] 82-82: expected , but instead found :
Remove :
(parse)
[error] 83-83: expected , but instead found }
Remove }
(parse)
[error] 83-83: Expected a statement but instead found ')) ?? []'.
Expected a statement here.
(parse)
[error] 110-110: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 110-110: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 124-124: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 125-125: Expected a statement but instead found 'catch (error)'.
Expected a statement here.
(parse)
[error] 127-127: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 127-127: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 136-136: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 139-139: expected } but instead the file ends
the file ends here
(parse)
[error] 21-139: Illegal use of an export declaration not at the top level
move this declaration to the top level
(parse)
[error] 139-139: expected } but instead the file ends
the file ends here
(parse)
packages/create-bot/templates/slack-bot/src/listeners/messages/index.ts
[error] 3-3: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 3-3: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 5-5: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 5-6: Illegal use of an import declaration not at the top level
move this declaration to the top level
(parse)
[error] 6-7: Illegal use of an import declaration not at the top level
move this declaration to the top level
(parse)
[error] 9-9: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 9-9: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 17-17: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 48-48: Expected a property, a shorthand property, a getter, a setter, or a method but instead found '{#if isAi'.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
[error] 48-48: expected , but instead found }
Remove }
(parse)
[error] 17-50: Illegal use of an export declaration not at the top level
move this declaration to the top level
(parse)
[error] 53-53: Expected an identifier, a member name, or a rest pattern but instead found '{#if isAi'.
Expected an identifier, a member name, or a rest pattern here.
(parse)
[error] 53-53: expected , but instead found }
Remove }
(parse)
[error] 54-54: expected , but instead found client
Remove client
(parse)
[error] 55-55: Expected an identifier, a member name, or a rest pattern but instead found '{/if'.
Expected an identifier, a member name, or a rest pattern here.
(parse)
[error] 55-55: expected , but instead found }
Remove }
(parse)
[error] 56-56: expected , but instead found say
Remove say
(parse)
[error] 62-62: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 62-62: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 67-67: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 69-69: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 69-69: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 79-79: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 106-106: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 106-106: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 120-120: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 121-121: Expected a statement but instead found 'catch (error)'.
Expected a statement here.
(parse)
[error] 125-125: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 125-125: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 135-135: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 148-148: expected } but instead the file ends
the file ends here
(parse)
packages/create-bot/templates/slack-bot/src/listeners/events/app-mention.ts
[error] 3-3: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 3-3: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 5-5: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 5-6: Illegal use of an import declaration not at the top level
move this declaration to the top level
(parse)
[error] 6-7: Illegal use of an import declaration not at the top level
move this declaration to the top level
(parse)
[error] 7-8: Illegal use of an import declaration not at the top level
move this declaration to the top level
(parse)
[error] 34-34: Expected a property, a shorthand property, a getter, a setter, or a method but instead found '{#if isAi'.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
[error] 34-34: expected , but instead found }
Remove }
(parse)
[error] 11-35: Illegal use of an export declaration not at the top level
move this declaration to the top level
(parse)
[error] 38-38: Expected an identifier, a member name, or a rest pattern but instead found '{#if isAi'.
Expected an identifier, a member name, or a rest pattern here.
(parse)
[error] 38-38: expected , but instead found }
Remove }
(parse)
[error] 39-39: expected , but instead found client
Remove client
(parse)
[error] 40-40: Expected an identifier, a member name, or a rest pattern but instead found '{/if'.
Expected an identifier, a member name, or a rest pattern here.
(parse)
[error] 40-40: expected , but instead found }
Remove }
(parse)
[error] 41-41: expected , but instead found say
Remove say
(parse)
[error] 46-46: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 46-46: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 50-50: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 52-52: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 52-52: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 60-60: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 77-77: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 77-77: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 89-89: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 90-90: Expected a statement but instead found 'catch (error)'.
Expected a statement here.
(parse)
[error] 92-92: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 92-92: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 103-103: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 106-106: expected } but instead the file ends
the file ends here
(parse)
🔇 Additional comments (48)
apps/ui/src/lib/simulator-settings.ts (4)
1-10: LGTM!The updated comment clearly documents the dual purpose of simulator settings and explains the merge behaviour with bot-specific settings.
25-31: LGTM!The default
ai_providervalue of 'openai' is sensible and aligns with the first option in the schema. API keys are correctly omitted from defaults.
37-72: LGTM!The conditional schema logic is well-structured. The
conditionobjects correctly tie each API key's requirement to its corresponding provider selection. The naming convention (${provider}_api_key) aligns with thegetApiKeyfunction insettings-store.ts.
112-116: LGTM!Placing the AI Provider group at order 0 appropriately prioritises it in the settings UI, as users typically need to configure their API credentials first.
packages/create-bot/templates/slack-bot/src/config/http-server.ts.tmpl (3)
11-18: Interface retainsmodel_tiersfor schema compatibility.The interface keeps the
model_tiersfield whilst the runtime value is hardcoded to{}. This maintains backwards compatibility with consumers expecting this shape. Ifmodel_tiersis being fully deprecated, consider adding a brief comment explaining this is retained for compatibility, or mark it as deprecated if appropriate.
43-51: LGTM!The hardcoded empty
model_tiersaligns with the PR's simplification of the model selection flow. The response structure remains compatible with existing consumers.
57-95: Well-implemented random port allocation for multi-bot support.The changes correctly address port conflict issues when running multiple bots:
- Using
port: 0allows the OS to assign an available port.- Explicit
127.0.0.1binding ensures IPv4 compatibility with Electron.- Returning the server instance (with
server.portaccessible) allows callers to discover the actual port.- Non-fatal error handling is appropriate since the config server is auxiliary.
Based on learnings, the permissive CORS configuration is acceptable for this Electron/localhost-only application.
apps/ui/src/lib/dispatcher.svelte.ts (2)
23-24: LGTM!Clean import of the centralised
isBotUserIdutility function to support multi-bot detection.
303-313: LGTM!Good refactor to use the centralised
isBotUserIdfunction. This correctly delegates bot detection to the state module, which handles both the legacyU_BOTformat and the new multi-botU_{botId}format. The comment accurately documents the expected formats.apps/ui/src/lib/config-client.ts (1)
73-94: LGTM!The function now correctly:
- Validates that
botIdis provided- Restricts usage to Electron mode only
- Returns
nullconsistently for all failure pathsThe previous concern about falling through to a web fetch path is no longer applicable since that code path has been removed.
apps/ui/src/lib/state.svelte.ts (4)
165-171: LGTM!Clean implementation for deriving the bot name from the DM channel ID. The pattern
D_{botId}is correctly parsed, and the fallback tosimulatorState.botNamehandles cases where the bot isn't in the connected bots map.
190-206: LGTM!Good defensive handling:
- Empty
channelIdreturns null early- DM channels (
D_prefix) are validated at runtime rather than against a static list, which aligns with the dynamic multi-bot support- Regular channels still require presence in the
CHANNELSarray
489-504: Intentional design noted for display purposes.The blanket
return trueon line 500 for anyU_prefix is intentional per the comment—it ensures messages from disconnected bots still render with bot styling. The check on line 497 (connectedBots.has(botId)) is effectively redundant since the function returnstrueregardless, but it does provide a semantic distinction between known and unknown bots.If you ever need to distinguish "definitely a bot" from "possibly a bot", consider returning a more nuanced result (e.g.,
{ isBot: true, isKnown: boolean }).
508-520: LGTM!The bot lookup logic correctly handles both the legacy
BOT_USER_IDcase and the new multi-botU_{botId}format. UsingArray.from(...)[0]for the legacy case is acceptable given the expected small size of theconnectedBotsmap.apps/ui/src/components/Message.svelte (2)
12-12: LGTM!Clean import of the centralised bot detection utilities from the state module.
49-57: LGTM!Good refactor to use the centralised utilities:
isBotUserIdcorrectly determines if the message is from a botgetBotByUserIdenables dynamic bot name resolution for multi-bot support- The fallback chain (
bot?.name ?? simulatorState.botName) ensures a display name is always availableapps/electron/scripts/compile-bots.ts (3)
10-22: LGTM!The type definitions are clean and well-structured.
BotEntrysupporting both string shorthand and object form provides good flexibility for configuration.
28-37: LGTM!The function correctly handles the case where
config.yamldoesn't exist and safely accesses nested properties with optional chaining.
39-63: LGTM!Good error messaging when bot name cannot be determined, and the resolution logic is clear.
apps/electron/electron.js (9)
49-49: LGTM!Good refactor to track both the process and its config port, enabling proper IPC-based config fetching.
238-260: LGTM!Clean migration from YAML to JSON manifest reading. Good fallback to discovery mode when no manifest exists, and proper error handling.
267-296: LGTM!The function correctly filters out bots whose binaries don't exist, preventing startup failures from missing files.
550-582: LGTM!The bot process management now correctly tracks both the process and its config port. The exit handler properly cleans up the map entry.
639-649: LGTM!Pushing settings to the emulator after it's ready ensures external bots receive configuration on registration. Good use of non-blocking approach with warning on failure.
729-732: LGTM!Correct destructuring to extract the process from the new object structure.
822-825: LGTM!Exposing
botPortsmapping enables the UI/preload to know which port each bot's config server is on.
835-891: Well-structured IPC handler with appropriate retry logic.The approach of querying the emulator first (since bots use random ports) with fallback to the local process map is correct. The retry loop handles the timing issue where the config server may not be immediately available.
Consider adding a small optimisation: if the emulator query succeeds but returns no
configPort, there's no point retrying the local map lookup in subsequent iterations.
1093-1107: Verify the async-to-quit pattern in thebefore-quithandler.The
before-quithandler prevents duplicate cleanup via theisQuittingflag, then callsapp.quit()after awaitingstopBackend(). Confirm whether this pattern reliably completes the async cleanup before application termination and whether Electron re-triggers the handler after the secondapp.quit()call. Ensure backend processes are fully stopped in practice.packages/create-bot/templates/slack-bot/src/listeners/events/app-mention.ts (1)
37-106: LGTM on theprocessMentionimplementation.The function has appropriate error handling with try/catch, proper reaction management for the AI path, and correctly builds the thread context for response generation.
packages/create-bot/templates/slack-bot/src/listeners/assistant/message.ts (2)
30-47: LGTM on message validation.The validation correctly filters out invalid message shapes, and the conditional variable extraction for AI mode is well-structured.
67-91: LGTM on thread history construction.The history mapping correctly assigns roles based on
bot_idpresence, and the context construction is appropriate. Based on learnings, the intentional lack of pagination for thread history is acceptable for this use case.packages/create-bot/templates/slack-bot/src/listeners/messages/index.ts (3)
18-46: LGTM on the DM filtering logic.The event handler correctly filters for DMs only, excludes bot messages and subtypes, and properly extracts thread context for reply handling.
80-86: Consider consistency: DM handler doesn't fetch thread history.Unlike the assistant handler, this DM handler initialises
historyas an empty array. If a user replies in a thread, the AI won't have context from previous messages.If thread context is desired for DM threads, consider fetching
conversations.repliessimilar to the assistant handler. Otherwise, if this is intentional for DMs to be stateless, this is fine.
136-147: Good user-friendly error messaging.The error classification provides helpful feedback for common failure modes (API key issues, rate limits). This improves the user experience compared to a generic error message.
packages/slack/src/server/types.ts (3)
1-8: No review note for comment-only edits.
102-135: Payload typings look good and clearly emulator-focused.Clear, minimal shapes for slash commands and message shortcuts.
313-322: Clean extraction of reactions request/response types.packages/slack/src/server/web-api.ts (11)
58-84: Nice, encapsulated token parsing helper.
Centralising bot ID extraction keeps multi-bot routing clean and easy to maintain.
181-237: Token propagation through Web API handlers is consistent.
Nice clean-through of the token parameter across endpoints.
262-273: auth.test now reflects the token-derived bot identity.
Looks aligned with the multi-bot goals.
326-329:chat.updatetoken signature change looks tidy.
No concerns here.
358-384: Reactions now attributed to the correct bot.
Good use of token-derived bot identity.
386-417: Reaction removals are now token-aware.
Behaviour aligns with multi-bot attribution.
598-672: files.uploadV2 now records the bot identity.
This keeps file messages consistent with the caller.
711-872: files.completeUploadExternal associates files with the correct bot.
Looks consistent with the new token-aware model.
1105-1169: User message logging and targeted app_mention dispatch look good.
Nice to see bot‑specific routing in mentions.
1242-1247: Returning simulator settings on config register is useful.
Helps external bots initialise cleanly.
1265-1274: ExposingconfigPortin connected bots is a helpful addition.
Good for IPC discovery.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
packages/create-bot/templates/slack-bot/src/listeners/events/app-mention.ts
Show resolved
Hide resolved
- OpenAI: gpt-4o → gpt-5.2 - Anthropic: claude-sonnet-4-20250514 → claude-sonnet-4-5 - Google: gemini-2.0-flash → gemini-3-flash
Bots choose their own config server port (random) and register it with the emulator. Electron now only queries the emulator for the config port instead of assuming PORT+1 or keeping a local fallback map.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/create-bot/templates/slack-bot/src/settings.ts.tmpl`:
- Around line 77-87: In getModel(), update the Google model identifier used in
the google(...) call from 'gemini-3-flash' to the correct API ID
'gemini-3-flash-preview' so the google(...) branch returns
google('gemini-3-flash-preview'); locate the getModel function in
settings.ts.tmpl and change the string inside the google(...) call accordingly.
🧹 Nitpick comments (1)
apps/electron/electron.js (1)
831-876: Consider validatingbotIdinput.The handler proceeds without validating
botId. While the code handles undefined gracefully, adding an early guard would make the contract explicit and fail fast.♻️ Suggested improvement
ipcMain.handle('bot:fetchConfig', async (_event, botId) => { + if (!botId || typeof botId !== 'string') { + electronLogger.warn({ botId }, 'Invalid botId provided to bot:fetchConfig') + return null + } + let configPort = null
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/electron/electron.jspackages/create-bot/templates/slack-bot/src/settings.ts.tmpl
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: tyom
Repo: tyom/botarium PR: 1
File: apps/ui/src/index.html:6-9
Timestamp: 2026-01-14T18:55:11.911Z
Learning: The Botarium application is designed exclusively for Electron or localhost deployment with no plans for public/production deployment, so security configurations like Content-Security-Policy can be optimized for local development rather than production hardening.
📚 Learning: 2026-01-14T21:36:12.466Z
Learnt from: tyom
Repo: tyom/botarium PR: 1
File: apps/ui/src/components/MessagePanel.svelte:10-23
Timestamp: 2026-01-14T21:36:12.466Z
Learning: The Botarium UI is a client-only Vite + Svelte application with no server-side rendering; it runs exclusively in Electron or browser environments, so checks for SSR-unsafe browser APIs like sessionStorage are unnecessary.
Applied to files:
apps/electron/electron.js
🔇 Additional comments (12)
apps/electron/electron.js (9)
19-19: LGTM!The
netimport supports the new port allocation mechanism, and wrapping the process in an object{ process }enables future extensibility for per-bot metadata.Also applies to: 49-49
187-202: LGTM - past concern resolved.This is a standard approach for OS-assigned port allocation. The previous review flagged a potential issue with
configPort = botPort + 1, but per the PR objectives, bots now independently allocate and register their own config server port with the emulator, making that concern obsolete.
238-260: LGTM!The manifest-based bot discovery with graceful fallback to discovery mode when no manifest exists is well-implemented.
267-296: LGTM!The manifest-driven configuration with existence validation is robust.
299-346: LGTM!The enhanced logging improves observability without exposing sensitive values—only field names are logged.
550-580: LGTM!The extended signature cleanly integrates per-bot port allocation, and the environment variable propagation is correct.
637-662: LGTM!The settings push to the emulator includes environment variables with sensitive data, but this is acceptable for local-only deployment. The per-bot port allocation with logging aids debugging.
727-727: LGTM!Destructuring correctly matches the updated
botProcessesMap value structure.
1078-1092: LGTM!The guarded quit flow correctly prevents double cleanup using the
isQuittingflag. The asyncbefore-quitpattern withevent.preventDefault()followed byapp.quit()after cleanup is the standard Electron approach for async shutdown.packages/create-bot/templates/slack-bot/src/settings.ts.tmpl (3)
2-10: LGTM!The conditional import blocks correctly gate each AI SDK import based on the selected provider, ensuring only the required dependency is included in the generated code.
16-55: LGTM!The environment schema correctly handles both production and local simulator modes. The per-provider API key validation blocks are well-structured, and the relaxed defaults for local mode align with the local-only deployment design.
59-73: LGTM!The settings loading function provides clear error messages for misconfigured environment variables, which will help developers troubleshoot setup issues.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Use 'gemini-3-flash-preview' instead of 'gemini-3-flash' to match the correct API model ID.
Wrap JSON.parse in try-catch to provide a clear error message with the config path instead of crashing with an unhelpful exception.
Return early with a warning when getElectronAPI() is falsy instead of falling through silently. Also log caught errors for better visibility.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/electron/scripts/compile-bots.ts`:
- Around line 86-98: The current loop that builds the bots array via
resolveBotEntry(entry) can produce multiple ResolvedBot objects with the same
name, causing later compilation outputs (binary and manifest) to be silently
overwritten; add a duplicate-name check after resolving each entry (use
botEntries, resolveBotEntry, and the bots array/ResolvedBot.name) to detect if a
resolved.name already exists, log a clear error listing the duplicate name and
the conflicting entries (or exit non-zero), and abort processing so duplicates
are fixed before compiling to avoid clobbering outputs.
- Around line 108-110: The manifest currently records bot.entry which can be
undefined; change the code that builds the manifest to record the resolved entry
variable (entry) or the compiled output path (e.g., entryPath or compiled binary
path) instead of bot.entry so downstream consumers always get a concrete entry
value; update wherever manifest fields are set (the code handling
entry/entryPath and the manifest write logic around functions/variables entry,
sourcePath, entryPath and the compile result) to use the resolved entry or
compiled path consistently (also apply the same change to the other manifest
assignment block referenced around the later compile steps).
In `@packages/create-bot/templates/slack-bot/src/settings.ts.tmpl`:
- Around line 34-49: Add configurable per-provider model environment variables
and validate provider model IDs instead of hard-coding them: introduce settings
keys like OPENAI_MODEL, ANTHROPIC_MODEL, GOOGLE_MODEL (with isLocalMode sensible
defaults) alongside OPENAI_API_KEY, ANTHROPIC_API_KEY, GOOGLE_API_KEY; update
AI_PROVIDER handling to select and validate the matching model env (e.g., check
model ID format or membership in an allowed list) before returning settings;
ensure the code paths that reference the hard-coded model IDs now read from
these new settings (symbols to update: AI_PROVIDER, OPENAI_API_KEY,
OPENAI_MODEL, ANTHROPIC_API_KEY, ANTHROPIC_MODEL, GOOGLE_API_KEY, GOOGLE_MODEL)
and surface clear validation errors when a provider’s model value is missing or
invalid.
🧹 Nitpick comments (1)
apps/electron/scripts/compile-bots.ts (1)
39-63: Validate bot names to avoid unsafe output paths.
nameis later used to build output paths; allow‑listed characters (e.g., letters, digits, dots, dashes, underscores) would prevent path traversal or accidental nested output.🧹 Suggested hardening
if (!name) { console.error(`Error: Cannot determine bot name for ${source}`) console.error(` No 'name' override provided and no simulator.id found in config.yaml`) return null } + const isSafeName = /^[A-Za-z0-9._-]+$/.test(name) + if (!isSafeName) { + console.error(`Error: Invalid bot name "${name}" (only letters, digits, . _ - allowed)`) + return null + } return {
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/electron/scripts/compile-bots.tspackages/create-bot/templates/slack-bot/src/app.ts.tmplpackages/create-bot/templates/slack-bot/src/settings.ts.tmplpackages/slack/src/server/socket-mode.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-16T23:29:55.585Z
Learnt from: tyom
Repo: tyom/botarium PR: 2
File: packages/create-bot/templates/slack-bot/src/slack/handlers.ts.tmpl:99-110
Timestamp: 2026-01-16T23:29:55.585Z
Learning: In the Slack bot template's handler (`packages/create-bot/templates/slack-bot/src/slack/handlers.ts.tmpl`), the `hasBotParticipatedInThread` function is a fallback path that only runs after bot restarts. The in-memory `threadTracker` handles active threads. The function intentionally limits to 100 messages to avoid multiple API calls and rate limit pressure for the rare edge case of threads with 100+ messages, where the failure mode is minor (bot won't auto-respond until re-mentioned).
Applied to files:
packages/create-bot/templates/slack-bot/src/app.ts.tmpl
🧬 Code graph analysis (2)
packages/slack/src/server/socket-mode.ts (2)
packages/slack/src/lib/logger.ts (1)
socketModeLogger(35-35)packages/slack/src/server/types.ts (1)
SlackEvent(225-233)
apps/electron/scripts/compile-bots.ts (1)
apps/electron/electron.js (2)
content(253-253)manifestPath(243-245)
🔇 Additional comments (17)
packages/slack/src/server/socket-mode.ts (6)
33-96: Heartbeat cadence and ping payload look sound.Clearer heartbeat behaviour and observability; no concerns here.
186-199: Immediate ping on open is a good safeguard.Helps avoid early client timeout races.
211-215: Debug logging is appropriately scoped.The debug payload is minimal and safe.
307-415: Targeted dispatch routing is clear and well-guarded.The DM and per-bot targeting paths read cleanly and fail safely.
466-469: dispatchMessageEvent logging looks good.Concise metadata is sufficient here.
490-503: Targeted app_mention dispatch is correctly wired.The new parameter is passed through cleanly.
packages/create-bot/templates/slack-bot/src/app.ts.tmpl (8)
1-16: LGTM!Imports are well-organised, and the simulator token generation using the bot's unique ID enables proper multi-bot support. Module-level state for connection tracking is appropriate for this startup pattern.
18-45: LGTM!The polling approach with a 100ms interval is efficient for detecting WebSocket readiness. Swallowing fetch errors is appropriate here since the emulator may not be ready during initial polling attempts.
47-73: LGTM!Clean separation between simulator and live mode configuration. The fail-fast behaviour when tokens are missing is appropriate, and the log level mapping is sensible.
82-146: LGTM! Guard forconfigServerPortproperly implemented.The registration logic with differentiated retry strategies (fast for transient WebSocket issues, slower for other errors) is well-designed. The guard at lines 86-89 correctly prevents registration when the config server port is unavailable.
148-160: LGTM!Startup sequence is clean. The conditional config server start and proper port capture integrates well with the guard in
registerWithSimulator.
162-193: LGTM! Reconnection handling is robust.The type assertion to access the internal receiver client is a known limitation of Bolt's type definitions. The reconnection flow correctly re-registers with the simulator to restore state after disconnections.
195-226: LGTM!Proper startup sequencing ensures WebSocket is tracked before registration. Graceful shutdown handling for both SIGTERM and SIGINT is correct for containerised and interactive environments.
228-231: LGTM!Top-level error handling correctly logs fatal errors and exits with a non-zero code, enabling proper process supervision and container orchestration behaviour.
apps/electron/scripts/compile-bots.ts (3)
10-22: Type definitions are clear and sufficient.
66-83: Good improvement: JSON parse failures now report clearly.
28-37: I need the review comment to rewrite. Please provide the review comment that needs to be rewritten within<review_comment>tags, and I will output the result in the required format.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Move streamer.stop() into a finally block to ensure it's always called, even if responseHandler.generateResponse throws inside the for-await loop.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/create-bot/templates/slack-bot/config.yaml.tmpl (1)
31-96: Addrequired_whenconditions to API key fields tied toai_provider.The API key fields lack conditional requirement logic. Whilst the UI disables model selection until an API key is entered, there is no form-level validation preventing the config from being saved with a provider selected but no corresponding API key. This would cause runtime failure when the bot attempts to use the missing key. The schema infrastructure supports
required_whenconditions (as seen inDynamicSettings.sveltelines 378–382), so addrequired_when: { field: 'ai_provider', equals: '<provider>' }to each API key field (e.g.required_when: { field: 'ai_provider', equals: 'openai' }foropenai_api_key).packages/slack/src/server/state.ts (1)
607-640: Guard against duplicate app IDs overwriting active bots.Line 607 only reuses bots in
disconnected/connectingstate. If the same app ID connects while an existing bot is stillconnected, the new entry can overwriteconnectedBots(Line 639-640) and leave a staleconnectionToBotId, which can misroute events and disconnect the live bot when the old socket closes. Consider treating a connected bot with the same app ID as a reconnect (clean old mapping) or rejecting the new registration.🛠️ Suggested adjustment to prevent stale mappings
- const existingBot = Array.from(this.connectedBots.values()).find((bot) => { - if (bot.status !== 'disconnected' && bot.status !== 'connecting') - return false - - // Prefer matching by id if both have one - if (newBotId && bot.appConfig.app?.id) { - return bot.appConfig.app.id === newBotId - } - - // Fall back to name matching for backward compatibility - return bot.appConfig.app.name === appConfig.app.name - }) + const existingBot = Array.from(this.connectedBots.values()).find((bot) => { + // Prefer matching by id if both have one (even if currently connected) + if (newBotId && bot.appConfig.app?.id) { + return bot.appConfig.app.id === newBotId + } + + if (bot.status !== 'disconnected' && bot.status !== 'connecting') + return false + + // Fall back to name matching for backward compatibility + return bot.appConfig.app.name === appConfig.app.name + }) + + if ( + existingBot?.status === 'connected' && + existingBot.connectionId !== connectionId + ) { + stateLogger.warn( + `Replacing active connection for ${existingBot.appConfig.app.name} (${existingBot.id})` + ) + this.connectionToBotId.delete(existingBot.connectionId) + }
🤖 Fix all issues with AI agents
In `@packages/create-bot/src/index.ts`:
- Around line 61-69: The printed "cd" step can be empty when
path.relative(process.cwd(), targetDir) returns an empty string; update the
logic around the relativePath calculation so that after computing relativePath
you fallback to '.' (current directory) when relativePath is falsy (empty
string) and then pass that safe value into printStep(1, `cd ${relativePath}`);
reference the symbols relativePath, targetDir, path.relative, process.cwd(), and
printStep to locate where to apply the fix.
In `@packages/create-bot/templates/slack-bot/src/settings.ts.tmpl`:
- Around line 151-152: The code passes settings.OPENROUTER_API_KEY (which is
optional) directly into createOpenRouter in the 'openrouter' branch, which can
be undefined; fix by validating or asserting the key before calling
createOpenRouter: check settings.OPENROUTER_API_KEY and throw a clear error (or
run the earlier validation step so it is guaranteed non-null) before invoking
createOpenRouter(modelId), or only call createOpenRouter with a non-null key
(e.g., use a validated/requiredSettings value) so the SDK is initialized with a
definite API key.
♻️ Duplicate comments (4)
packages/create-bot/templates/slack-bot/src/app.ts.tmpl (1)
29-37: Multi-bot race condition remains:connected_bots > 0doesn't confirm THIS bot's connection.As previously noted, in multi-bot scenarios another bot's existing connection satisfies this check immediately, potentially allowing registration to proceed before this bot's WebSocket is tracked. Consider capturing the initial
connected_botscount before polling and waiting for an increment, or rely on the server'sno_websocket_connectionerror as the authoritative check.packages/create-bot/templates/slack-bot/src/settings.ts.tmpl (1)
134-139: Hardcoded model defaults contain deprecated identifiers.The
DEFAULT_MODELSvalues have known deprecation concerns:
gemini-2.0-flash: Deprecated, scheduled for shutdown Feb 2026; usegemini-2.5-flashgpt-4o: Snapshot aliases being retired; consider pinning to a stable snapshotclaude-sonnet-4-5: Missing version timestamp; useclaude-sonnet-4-5-20250929Since
MODEL_DEFAULTnow allows overrides, this is mitigated but the defaults should still reflect current best practices.apps/ui/src/components/DynamicSettings.svelte (2)
529-595: Validation status won’t reset after editing an invalid key.
Invalid states store novalidatedValue, andgetValidationStatusonly resets when the stored value is truthy. This leaves the “invalid” icon stuck after edits.Suggested fix
if (!value || value.trim() === '') { - apiKeyValidation[key] = { status: 'invalid', error: 'API key is empty' } + apiKeyValidation[key] = { + status: 'invalid', + error: 'API key is empty', + validatedValue: value, + } return } @@ .catch(() => { apiKeyValidation[key] = { status: 'invalid', error: 'Failed to fetch model tiers', - validatedValue: undefined, + validatedValue: value, } }) } else { - apiKeyValidation[key] = { status: 'invalid', error: result.error, validatedValue: undefined } + apiKeyValidation[key] = { + status: 'invalid', + error: result.error, + validatedValue: value, + } } } catch { - apiKeyValidation[key] = { status: 'invalid', error: 'Validation failed', validatedValue: undefined } + apiKeyValidation[key] = { + status: 'invalid', + error: 'Validation failed', + validatedValue: value, + } } @@ - if ( - validation.validatedValue && - validation.validatedValue !== currentValue - ) { + if ( + validation.validatedValue !== undefined && + validation.validatedValue !== currentValue + ) { return 'idle' }
126-135: Re-enter loading state on bot changes to avoid stale edits.
WhenbotIdchanges you start a new fetch but keeploadingfalse, so users can edit the previous bot’s settings while the new config is in flight. Consider flippingloadingback totruewhen a new key is detected.Suggested tweak
- lastFetchedKey = currentKey + lastFetchedKey = currentKey + loading = true
🧹 Nitpick comments (14)
packages/create-bot/templates/slack-bot/.env.example.tmpl (2)
1-6: Clean Slack configuration structure.The placeholder values appropriately follow Slack's token format conventions (e.g.,
xoxb-for bot tokens,xapp-for app tokens), making it clear what type of credentials are expected.💡 Optional: Add documentation links
Consider adding comments with links to where users can obtain these credentials:
# Slack Configuration +# Get your tokens from: https://api.slack.com/apps SLACK_BOT_TOKEN=xoxb-your-bot-token SLACK_APP_TOKEN=xapp-your-app-token SLACK_SIGNING_SECRET=your-signing-secret
7-21: Well-structured AI configuration with good security defaults.All API keys are commented out by default, which is excellent security practice. The multi-provider support is clearly documented.
💡 Optional: Add model examples
The
MODEL_DEFAULTcomment could include examples of valid model names for each provider to guide users:# Model Selection (optional - override provider defaults) +# Examples: gpt-4, claude-3-5-sonnet-20241022, gemini-pro, meta-llama/llama-3.1-8b-instruct # MODEL_DEFAULT=packages/create-bot/src/index.ts (1)
69-71: Avoid instructing .env configuration when no vars are required.Step 2 always tells users to configure
.env, even whengetRequiredEnvVars().hasAnyis false. Consider conditionally showing the step or replacing it with “No environment variables required” to reduce confusion.Also applies to: 78-101
packages/create-bot/templates/slack-bot/src/app.ts.tmpl (2)
108-119: Consider: fast retries consume the retry budget quickly.When
no_websocket_connectionoccurs, the 200ms delay is used but the attempt counter still increments. This means all 10 retries could be exhausted in ~2 seconds if the WebSocket remains slow to track. If this is intentional (fail fast on persistent issues), this is fine. Otherwise, consider not incrementingattemptfor this transient condition, or using a separate budget for fast retries.
238-249: Consider: config server cleanup on shutdown.The graceful shutdown stops the Slack app but doesn't explicitly stop the config server. While the process exit will clean up the socket, explicitly closing the server ensures cleaner shutdown and quicker port release, especially during rapid restart cycles.
♻️ Suggested improvement
If
startConfigServer()returns a server object with aclose()method, consider:+let configServer: ReturnType<typeof startConfigServer> | undefined // In main(): if (isSimulatorMode) { - const server = startConfigServer() - if (server) { - configServerPort = server.port + configServer = startConfigServer() + if (configServer) { + configServerPort = configServer.port } } // In shutdown: const shutdown = async () => { appLogger.info('Shutting down...') + configServer?.close?.() await app.stop() process.exit(0) }packages/create-bot/templates/slack-bot/src/settings.ts.tmpl (2)
146-156: Missing API key validation before provider instantiation.The
getModel()function doesn't verify that the required API key exists for the selected provider before attempting to create the model instance. This will cause cryptic runtime errors when the SDK attempts to authenticate.Consider adding early validation:
♻️ Suggested improvement
export function getModel() { const provider = settings.AI_PROVIDER || 'openai' const modelId = settings.MODEL_DEFAULT || DEFAULT_MODELS[provider] || 'gpt-4o' + const requiredKeys: Record<string, string | undefined> = { + openai: settings.OPENAI_API_KEY, + anthropic: settings.ANTHROPIC_API_KEY, + google: settings.GOOGLE_API_KEY, + openrouter: settings.OPENROUTER_API_KEY, + } + + if (!requiredKeys[provider]) { + throw new Error(`Missing API key for provider "${provider}". Set the corresponding environment variable.`) + } + switch (provider) { case 'anthropic': return anthropic(modelId)
61-69: Silent error handling infetchSimulatorSettingsis appropriate but could benefit from debug logging.The empty catch block silently swallows connection errors when the emulator is unavailable. While this fallback behaviour is intentional, adding a debug-level log would aid troubleshooting when settings aren't being fetched as expected.
♻️ Optional improvement
} catch { - // Emulator not available, use env vars only + // Emulator not available, use env vars only + if (process.env.LOG_LEVEL === 'debug') { + console.debug('[settings] Simulator settings endpoint unavailable, using env vars only') + } }packages/create-bot/src/scaffold.ts (1)
137-175: Consider removingasyncfrom functions that don't useawait.Both
copyFileandcopyDirectoryare declaredasyncbut only use synchronousfsmethods (readFileSync,writeFileSync,copyFileSync,readdirSync,mkdirSync). This makes theasynckeyword misleading.Either:
- Remove
asyncand change return types tovoid(simpler, matches current behaviour)- Convert to actual async operations using
fs/promises(better for large templates)Option 1: Remove async (simpler)
-async function copyFile( +function copyFile( src: string, dest: string, ctx: TemplateContext -): Promise<void> { +): void { if (isTemplateFile(src)) { const content = fs.readFileSync(src, 'utf-8') const processed = processTemplate(content, ctx) fs.writeFileSync(dest, processed) } else { fs.copyFileSync(src, dest) } } -async function copyDirectory( +function copyDirectory( src: string, dest: string, ctx: TemplateContext, rules: SkipRules -): Promise<void> { +): void { const entries = fs.readdirSync(src, { withFileTypes: true }) for (const entry of entries) { if (shouldSkip(entry.name, entry.isDirectory(), rules, ctx)) { continue } const srcPath = path.join(src, entry.name) const destName = getDestinationName(entry.name) const destPath = path.join(dest, destName) if (entry.isDirectory()) { fs.mkdirSync(destPath, { recursive: true }) - await copyDirectory(srcPath, destPath, ctx, rules) + copyDirectory(srcPath, destPath, ctx, rules) } else { - await copyFile(srcPath, destPath, ctx) + copyFile(srcPath, destPath, ctx) } } }packages/create-bot/templates/slack-bot/config.yaml.tmpl (1)
100-110: Consider renaming the AI group label for clarity.The group now covers provider selection, keys, and model defaults; “AI Configuration” reads clearer than “AI Provider”.
♻️ Suggested tweak
- label: AI Provider + label: AI Configurationapps/ui/src/lib/backend-state.svelte.ts (1)
248-257: Consider using$state.snapshotfor consistency.This function uses
JSON.parse(JSON.stringify(settings))whileupdateSettinguses$state.snapshot(settings). Both achieve the same goal, but using$state.snapshothere would be more consistent and idiomatic for Svelte 5.♻️ Suggested refactor
- // Create plain object copy to avoid reactive proxy serialization issues - const settingsSnapshot = JSON.parse( - JSON.stringify(settings) - ) as typeof settings + // Create plain object snapshot to avoid reactive proxy serialization issues + const settingsSnapshot = $state.snapshot(settings)apps/electron/electron.js (2)
411-438: Consider logging when model IDs are reset due to provider mismatch.When
MODEL_FASTorMODEL_THINKINGare deleted due to provider incompatibility, this happens silently. Adding a debug log would help troubleshoot configuration issues.♻️ Suggested improvement
// Apply same logic to MODEL_FAST and MODEL_THINKING for consistency if (env.MODEL_FAST && !isModelCompatibleWithProvider(env.MODEL_FAST, provider)) { + electronLogger.debug({ model: env.MODEL_FAST, provider }, 'Clearing incompatible MODEL_FAST') delete env.MODEL_FAST // Let the bot use its default } if (env.MODEL_THINKING && !isModelCompatibleWithProvider(env.MODEL_THINKING, provider)) { + electronLogger.debug({ model: env.MODEL_THINKING, provider }, 'Clearing incompatible MODEL_THINKING') delete env.MODEL_THINKING // Let the bot use its default }
972-1000: Consider extracting retry constants.The retry logic is well-implemented with appropriate delays and attempts. Consider extracting
MAX_RETRIESandRETRY_DELAYto module-level constants for easier tuning and consistency with other retry logic in the file.apps/ui/src/components/DynamicSettings.svelte (1)
415-422: Treat env‑controlled API keys as present for model selection.
hasProviderApiKey()only checksformData, so when the key is set via env (and intentionally hidden), model selection stays disabled. Consider treating env‑overridden keys as “present”.Possible change
function hasProviderApiKey(): boolean { const provider = formData.ai_provider as string if (!provider) return false const keyField = `${provider}_api_key` const key = formData[keyField] as string | undefined - return !!key && key.length > 0 + return (!!key && key.length > 0) || isEnvOverridden(keyField) }packages/slack/src/server/state.ts (1)
1026-1133: Consider defensive copying of simulator settings.Line 1026-1035 stores and returns the same object reference, so external callers can mutate state without logging or validation. A shallow copy on set/get keeps encapsulation intact.
♻️ Optional hardening
setSimulatorSettings(settings: Record<string, unknown>): void { - this.simulatorSettings = settings + this.simulatorSettings = { ...settings } stateLogger.debug( { keys: Object.keys(settings) }, 'Simulator settings updated' ) } getSimulatorSettings(): Record<string, unknown> { - return this.simulatorSettings + return { ...this.simulatorSettings } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
apps/electron/electron.jsapps/ui/src/components/AppSettings.svelteapps/ui/src/components/DynamicSettings.svelteapps/ui/src/lib/backend-state.svelte.tsapps/ui/src/lib/config-client.tsapps/ui/src/lib/simulator-settings.tspackages/create-bot/bin/cli.tspackages/create-bot/src/index.tspackages/create-bot/src/prompts.tspackages/create-bot/src/scaffold.tspackages/create-bot/src/utils/env-vars.tspackages/create-bot/src/utils/prompt-options.tspackages/create-bot/src/utils/template.test.tspackages/create-bot/src/utils/template.tspackages/create-bot/templates/slack-bot/.env.example.tmplpackages/create-bot/templates/slack-bot/config.yaml.tmplpackages/create-bot/templates/slack-bot/package.json.tmplpackages/create-bot/templates/slack-bot/src/app.ts.tmplpackages/create-bot/templates/slack-bot/src/config/http-server.ts.tmplpackages/create-bot/templates/slack-bot/src/settings.ts.tmplpackages/slack/src/server/state.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/create-bot/src/utils/env-vars.ts
- packages/create-bot/src/utils/prompt-options.ts
- packages/create-bot/templates/slack-bot/src/config/http-server.ts.tmpl
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2026-01-16T23:30:02.205Z
Learnt from: tyom
Repo: tyom/botarium PR: 2
File: packages/create-bot/templates/slack-bot/src/slack/handlers.ts.tmpl:99-110
Timestamp: 2026-01-16T23:30:02.205Z
Learning: In the Slack bot template's handler (`packages/create-bot/templates/slack-bot/src/slack/handlers.ts.tmpl`), the `hasBotParticipatedInThread` function is a fallback path that only runs after bot restarts. The in-memory `threadTracker` handles active threads. The function intentionally limits to 100 messages to avoid multiple API calls and rate limit pressure for the rare edge case of threads with 100+ messages, where the failure mode is minor (bot won't auto-respond until re-mentioned).
Applied to files:
packages/create-bot/templates/slack-bot/src/app.ts.tmpl
📚 Learning: 2026-01-14T21:36:12.466Z
Learnt from: tyom
Repo: tyom/botarium PR: 1
File: apps/ui/src/components/MessagePanel.svelte:10-23
Timestamp: 2026-01-14T21:36:12.466Z
Learning: The Botarium UI is a client-only Vite + Svelte application with no server-side rendering; it runs exclusively in Electron or browser environments, so checks for SSR-unsafe browser APIs like sessionStorage are unnecessary.
Applied to files:
apps/electron/electron.js
📚 Learning: 2026-01-14T21:36:07.933Z
Learnt from: tyom
Repo: tyom/botarium PR: 1
File: apps/ui/src/components/MessagePanel.svelte:10-23
Timestamp: 2026-01-14T21:36:07.933Z
Learning: In the client-only Botarium UI (Vite + Svelte), do not add or rely on SSR-specific guards (e.g., checks for sessionStorage) in components. Since the app runs only in Electron or the browser, code that accesses browser APIs will execute in a non-SSR environment, so SSR-unsafe checks are unnecessary and can be removed for maintainability.
Applied to files:
apps/ui/src/components/AppSettings.svelteapps/ui/src/components/DynamicSettings.svelte
📚 Learning: 2026-01-16T19:57:22.904Z
Learnt from: tyom
Repo: tyom/botarium PR: 2
File: packages/create-bot/templates/slack-bot/src/db/index.ts.tmpl:12-23
Timestamp: 2026-01-16T19:57:22.904Z
Learning: In the create-botarium CLI scaffolding (packages/create-bot), the db/ directory is skipped entirely when dbAdapter === 'none' (see scaffold.ts shouldSkipDirectory). The isDb flag is true only when dbAdapter !== 'none', and when isDb is true, exactly one of isSqlite or isPostgres will be true based on the dbAdapter value. Therefore, template files in db/ are never rendered when no database is selected, and always have exactly one database adapter flag set when they are rendered.
Applied to files:
packages/create-bot/src/index.tspackages/create-bot/bin/cli.tspackages/create-bot/src/prompts.tspackages/create-bot/src/scaffold.tspackages/create-bot/src/utils/template.ts
🧬 Code graph analysis (7)
packages/create-bot/templates/slack-bot/package.json.tmpl (2)
packages/slack/src/index.ts (1)
start(19-24)apps/electron/scripts/compile-bots.ts (1)
main(53-109)
packages/slack/src/server/state.ts (2)
packages/slack/src/server/types.ts (1)
ConnectedBot(348-354)packages/slack/src/server/web-api.ts (2)
bot(1206-1213)getSimulatorConfig(1200-1203)
apps/ui/src/components/AppSettings.svelte (2)
apps/ui/src/lib/backend-state.test.ts (3)
globalSettings(36-52)test(3-53)settings(4-19)apps/ui/src/lib/state.svelte.ts (1)
setAppConfig(391-397)
packages/create-bot/src/index.ts (3)
packages/create-bot/src/utils/cli-output.ts (9)
printHeader(7-11)printCancelled(18-20)printStatus(13-16)printSuccess(22-26)printBlankLine(44-46)printStep(28-30)printCommand(32-34)printSectionHeader(40-42)printEnvVar(36-38)packages/create-bot/src/prompts.ts (2)
promptForSelections(124-156)UserSelections(18-24)packages/create-bot/src/utils/env-vars.ts (1)
getRequiredEnvVars(36-53)
packages/create-bot/templates/slack-bot/src/settings.ts.tmpl (3)
apps/ui/src/lib/backend-state.test.ts (1)
globalSettings(36-52)packages/create-bot/templates/slack-bot/src/ai/agents/base.ts (2)
AgentConfig(9-16)agent(22-73)packages/create-bot/templates/slack-bot/src/ai/chat.ts (1)
ChatContext(5-10)
packages/create-bot/src/utils/template.ts (1)
packages/create-bot/src/prompts.ts (2)
BotTemplate(16-16)DbAdapter(16-16)
apps/ui/src/lib/backend-state.svelte.ts (1)
apps/electron/electron.js (2)
settings(306-306)settings(1066-1066)
🪛 dotenv-linter (4.0.0)
packages/create-bot/templates/slack-bot/.env.example.tmpl
[warning] 5-5: [UnorderedKey] The SLACK_APP_TOKEN key should go before the SLACK_BOT_TOKEN key
(UnorderedKey)
[warning] 7-7: [IncorrectDelimiter] The {{#if isAi}} key has incorrect delimiter
(IncorrectDelimiter)
[warning] 7-7: [KeyWithoutValue] The {{#if isAi}} key should be with a value or have an equal sign
(KeyWithoutValue)
[warning] 7-7: [LeadingCharacter] Invalid leading character detected
(LeadingCharacter)
[warning] 7-7: [LowercaseKey] The {{#if isAi}} key should be in uppercase
(LowercaseKey)
[warning] 21-21: [IncorrectDelimiter] The {{/if}} key has incorrect delimiter
(IncorrectDelimiter)
[warning] 21-21: [KeyWithoutValue] The {{/if}} key should be with a value or have an equal sign
(KeyWithoutValue)
[warning] 21-21: [LeadingCharacter] Invalid leading character detected
(LeadingCharacter)
[warning] 21-21: [LowercaseKey] The {{/if}} key should be in uppercase
(LowercaseKey)
[warning] 22-22: [IncorrectDelimiter] The {{#if isPostgres}} key has incorrect delimiter
(IncorrectDelimiter)
[warning] 22-22: [KeyWithoutValue] The {{#if isPostgres}} key should be with a value or have an equal sign
(KeyWithoutValue)
[warning] 22-22: [LeadingCharacter] Invalid leading character detected
(LeadingCharacter)
[warning] 22-22: [LowercaseKey] The {{#if isPostgres}} key should be in uppercase
(LowercaseKey)
[warning] 22-22: [UnorderedKey] The {{#if isPostgres}} key should go before the {{/if}} key
(UnorderedKey)
[warning] 26-26: [DuplicatedKey] The {{/if}} key is duplicated
(DuplicatedKey)
[warning] 26-26: [IncorrectDelimiter] The {{/if}} key has incorrect delimiter
(IncorrectDelimiter)
[warning] 26-26: [KeyWithoutValue] The {{/if}} key should be with a value or have an equal sign
(KeyWithoutValue)
[warning] 26-26: [LeadingCharacter] Invalid leading character detected
(LeadingCharacter)
[warning] 26-26: [LowercaseKey] The {{/if}} key should be in uppercase
(LowercaseKey)
🔇 Additional comments (49)
packages/create-bot/templates/slack-bot/.env.example.tmpl (2)
1-26: Static analysis warnings are false positives.The dotenv-linter warnings can be safely ignored. This is a Handlebars template file (
.tmpl), not a standard.envfile, so the templating syntax ({{#if}},{{/if}},{{botNamePascal}}, etc.) is expected and correct.
22-26: Database configuration looks good.The use of
{{packageName}}for the database name ensures consistency with the package structure. The generic credentials are appropriate for an example file, as users will replace these when creating their actual.envfile.packages/create-bot/templates/slack-bot/src/app.ts.tmpl (7)
1-16: LGTM!Imports are well-organised and the simulator token generation using
config.simulator.idcorrectly enables multi-bot differentiation. Module-level state for connection tracking is appropriate for this use case.
47-73: LGTM!The
createAppfunction cleanly separates simulator and live mode configurations. The log level mapping is straightforward and sufficient for the current needs.
75-80: LGTM!The
RegistrationResponseinterface is well-defined and captures the expected response shape from the simulator registration endpoint.
82-95: Good improvement:configServerPortguard added.The early return when
configServerPortis undefined prevents registration with an invalid payload. This addresses the previously raised concern about IPC config fetching failures.
185-216: LGTM!The WebSocket lifecycle handling is well-implemented. The reconnection logic correctly re-registers with the simulator after verifying WebSocket tracking. The type assertion for accessing
receiver.clientis a reasonable workaround for Bolt's unexposed internal types.
218-236: LGTM!The startup sequence correctly waits for WebSocket tracking before registration, with sensible fallback behaviour. Proceeding with registration even when WebSocket isn't detected leverages the retry logic in
registerWithSimulatorfor resilience.
251-254: LGTM!Standard entry point pattern with proper error handling and logging for fatal errors.
packages/create-bot/templates/slack-bot/src/settings.ts.tmpl (2)
74-101: Settings loading logic is well-structured.The
loadSettings()function properly:
- Fetches simulator settings when in simulator mode
- Respects explicit
.envprovider choice by filtering out simulator API keys (lines 80-85)- Merges with correct precedence (env vars override simulator settings)
- Provides clear error messages for validation failures
103-105: ES module configuration is properly set.The template's
package.jsoncorrectly specifies"type": "module", andtsconfig.jsonis configured with"module": "Preserve"and"moduleDetection": "force", so theexport let settings = await loadSettings()pattern will work correctly in generated projects.packages/create-bot/src/utils/template.test.ts (3)
4-32: LGTM!The
toPascalCasetest suite covers a good range of input patterns including mixed separators and edge cases with numbers.
34-59: LGTM!The
toPackageNametests properly verify npm package name normalisation, including special character handling and hyphen collapsing.
61-113: LGTM!The
createTemplateContexttests cover the key scenarios including the mutual exclusivity of database adapters. Based on learnings, this aligns with the scaffolding behaviour whereisDbbeing true means exactly one ofisSqliteorisPostgreswill be true.packages/create-bot/src/prompts.ts (5)
1-16: LGTM!Clean refactoring to import from centralised utility modules and re-export types for external consumers.
18-31: LGTM!The distinction between
PartialSelections(raw string inputs) andUserSelections(validated typed outputs) is a good pattern for input validation.
33-78: LGTM!The
buildQuestionsfunction cleanly delegates to centralised utilities for choices and validation.
80-109: LGTM!The
mergeAnswersfunction properly validates inputs against the source of truth constants before returning typed selections.
111-156: LGTM!The prompt flow is well-structured with proper cancellation handling and target directory validation.
packages/create-bot/src/utils/template.ts (4)
3-14: LGTM!Single source of truth pattern with derived types is well implemented. The
AI_PROVIDERSconstant is exported for use in other modules (e.g., validation), whilstAdapterFlagscorrectly only derives database-related flags as per the template context needs.
16-41: LGTM!The
TemplateContextinterface is well-documented, andprocessTemplatecorrectly usesnoEscape: truesince the output is source code, not HTML.
43-75: LGTM!The
createTemplateContextfunction elegantly auto-derives boolean flags from the adapter constants. The type assertion on line 64 is necessary due to TypeScript'sObject.fromEntriesreturn type limitations.
77-94: LGTM!The string transformation utilities are well-implemented and have comprehensive test coverage in
template.test.ts.packages/create-bot/src/scaffold.ts (6)
15-40: LGTM!Constants are well-organised and clearly documented. The separation of AI-dependent and DB-dependent directories aligns with the learnings about conditional directory skipping.
45-58: LGTM!The
SkipRulesinterface provides a flexible mechanism for conditional file/directory exclusions using both static prefixes and dynamic function-based exclusions.
64-116: LGTM!The
buildSkipRulesfunction correctly implements conditional skipping. Based on learnings, whenisDbis true, exactly one ofisSqliteorisPostgreswill be true, making thesqlite.ts/postgres.tsexclusions mutually exclusive as intended.
122-135: LGTM!The compound extension handling in
isTemplateFilecorrectly identifies files like.env.exampleby reconstructing the extension from the basename.
181-214: LGTM!The
cleanJsonfunction handles trailing comma artifacts from Handlebars conditionals with a sensible fallback if parsing fails. ThecreateEnvFilefunction safely initialises the.envfile only when needed.
223-256: LGTM!The
scaffoldfunction is well-structured with clear steps: validate template, prepare target, build context and rules, copy files, and perform cleanup. ThermSyncfor overwrite is appropriate since the user has already confirmed this action upstream.packages/create-bot/bin/cli.ts (1)
6-41: LGTM! Provider option correctly removed from CLI.The removal of the
--provideroption aligns with the PR objective to make generated bots provider-agnostic at runtime. Provider selection is now handled viaAI_PROVIDERenvironment variable or Electron settings, which is a cleaner separation of concerns.apps/ui/src/lib/backend-state.svelte.ts (2)
198-208: Good defensive fix for the race condition.Setting
backendReady = falsebefore the IPC call prevents the race wherebackend:readyfires duringsaveSettings, which could leave the UI in an inconsistent state.
214-223: Correct use of$state.snapshotfor IPC serialisation.Using
$state.snapshot(settings)properly extracts a plain object from Svelte 5's reactive proxy, avoiding serialisation issues over IPC.packages/create-bot/templates/slack-bot/package.json.tmpl (2)
14-28: Dependency versions look appropriate.The AI SDK packages and Slack client dependencies have been updated. Previous review comments about version mismatches appear to have been addressed.
8-12: No action required. The entry point in the template (src/app.ts) aligns with the default incompile-bots.ts, which also defaults to'src/app.ts'. There is no mismatch between the template and the build configuration.Likely an incorrect or invalid review comment.
apps/electron/electron.js (4)
192-207: Good implementation of dynamic port allocation.Using
net.createServerwith port 0 lets the OS assign an available port, which is the correct approach for avoiding port conflicts.
350-357: DEFAULT_MODELS provides sensible fallbacks.The defaults align with the first model in each tier from MODEL_TIERS in the UI settings, ensuring consistency.
698-713: Good addition: pushing settings to emulator on startup.This ensures external bots receive global settings when they register, enabling proper configuration inheritance.
1206-1220: Good improvement to quit handling.The
isQuittingguard prevents double-cleanup which could cause issues during shutdown. The pattern of preventing immediate quit, cleaning up, then callingapp.quit()is correct.apps/ui/src/lib/config-client.ts (1)
74-98: Clean implementation with proper guards.The function now correctly:
- Validates
botIdis provided- Guards against non-Electron mode
- Checks
apiis available before use- Logs errors for debugging
The previous fallthrough bug has been properly addressed.
apps/ui/src/lib/simulator-settings.ts (4)
14-47: Well-structured MODEL_TIERS mapping.The tier structure (fast/default/thinking) provides a clear hierarchy for model selection across providers, and the OpenRouter models correctly use the
org/modelformat.
72-82: Good use ofas constfor type safety.Using
as constonBOT_OVERRIDABLE_SETTINGSensures the array elements are literal types, which helps with type checking when these settings are referenced elsewhere.
87-150: Conditional visibility for API keys is good UX.The
conditionfield ensures only the relevant API key field is shown based on the selected provider, reducing UI clutter and confusion.
186-208: Sensible group defaults.Having the AI Settings group expanded by default whilst Advanced is collapsed provides a good balance between discoverability and reduced visual noise.
apps/ui/src/components/DynamicSettings.svelte (1)
206-218: Deep‑merge ofmodel_tierslooks good.
Nice job preserving provider defaults while allowing per‑bot overrides.apps/ui/src/components/AppSettings.svelte (1)
38-119: Inheritance filtering and save filtering look consistent.
The logic cleanly separates inheritable simulator settings from bot‑specific fields and avoids persisting redundant overrides.packages/slack/src/server/state.ts (4)
58-60: LGTM: simulator settings slot is clear and scoped.
296-314: LGTM: per-bot identity helper is straightforward.
340-346: LGTM: D_ channels are treated as direct messages.
745-769: LGTM: reconnect flow avoids premature UI updates.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Compare connected_bots against initial count rather than zero, ensuring the health check waits for this specific bot's WebSocket connection rather than passing when other bots are already connected.
Prevents sending blank messages when responseHandler.generateResponse yields no content.
Use GPT-5.2, Gemini 3.0 preview, and Claude 4.5 as defaults. Remove hardcoded fallback and add error for unconfigured providers.
- Add fallback to '.' when relativePath is empty in printNextSteps - Add non-null assertion for OPENROUTER_API_KEY in template
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@packages/create-bot/templates/slack-bot/src/app.ts.tmpl`:
- Around line 186-191: The config HTTP server started in the isSimulatorMode
branch is only used to read server.port and the reference is discarded, so it
never gets closed on shutdown; update the code to keep the returned server
(e.g., const configServer = startConfigServer()) instead of discarding it,
assign configServerPort from configServer.port, and during shutdown alongside
app.stop() invoke configServer.close() (guarded by a null check) to ensure the
HTTP server is properly closed and the port released.
In `@packages/create-bot/templates/slack-bot/src/listeners/assistant/message.ts`:
- Around line 12-15: Add a ts?: string field to the ThreadMessage interface and,
after calling conversations.replies, filter the returned thread messages to
exclude the current message by comparing each reply's ts to the incoming
message.ts before constructing the prompt passed to generateResponse; update
references in the code that iterate thread messages (e.g., where ThreadMessage
is used and where conversations.replies results are mapped) so the current
message is omitted and duplication is prevented.
In `@packages/create-bot/templates/slack-bot/src/listeners/events/app-mention.ts`:
- Around line 28-33: The current stripping logic on event.text calls replace
with regexes built from slackConfig.app.name and slackConfig.app.id even when
those values are empty, which yields `/@?/gi` and strips all "@" characters; fix
by only performing the .replace(...) for app.name and app.id when those values
are non-empty (truthy) — e.g., compute text =
event.text.replace(/<@[A-Z0-9]+(\|[^>]*)?>/g, '').trim() and then conditionally
call .replace(new RegExp(...), '') using slackConfig.app.name and
slackConfig.app.id only if they are present (use the same escapeRegExp helper)
so escapeRegExp and the replace calls remain unchanged but guarded.
In `@packages/create-bot/templates/slack-bot/src/settings.ts.tmpl`:
- Around line 135-140: Update the DEFAULT_MODELS constant to use the correct
provider model IDs: in the DEFAULT_MODELS record (symbol DEFAULT_MODELS) change
the value for the "google" key from "gemini-3.0-flash-preview" to
"gemini-3-flash-preview" and change the value for the "openrouter" key from
"anthropic/claude-4.5-sonnet" to "anthropic/claude-sonnet-4.5"; leave the openai
and anthropic entries unchanged.
♻️ Duplicate comments (1)
packages/create-bot/templates/slack-bot/src/settings.ts.tmpl (1)
156-157: OpenRouter API key may beundefined.
settings.OPENROUTER_API_KEYis typed as optional and could beundefinedwhen passed tocreateOpenRouter(). This may cause unclear errors at request time rather than at initialisation.Consider adding validation before using the key:
🔧 Suggested fix
case 'openrouter': + if (!settings.OPENROUTER_API_KEY) { + throw new Error('OPENROUTER_API_KEY is required when using OpenRouter provider.') + } return createOpenRouter({ apiKey: settings.OPENROUTER_API_KEY })(modelId)
🧹 Nitpick comments (3)
packages/create-bot/templates/slack-bot/src/app.ts.tmpl (2)
143-147: Consider extracting hardcoded settings list to configuration.The
settingsToApplyarray is hardcoded here. If new simulator settings are added (e.g., additional provider keys), this list must be manually updated. Consider defining this list in a shared constant or deriving it from the settings schema to reduce maintenance burden.
197-198: Acknowledge: Internal API access is fragile but pragmatic.Accessing
app.receiver.clientviaanycast relies on Bolt's internal structure. This is acceptable for simulator diagnostics, but be aware it may break with Bolt version upgrades.packages/create-bot/templates/slack-bot/src/settings.ts.tmpl (1)
67-69: Consider logging errors at debug level for troubleshooting.The empty catch block silently discards all errors, which could make it difficult to diagnose issues when the emulator is running but returning unexpected responses or experiencing network problems.
♻️ Suggested improvement
} catch { - // Emulator not available, use env vars only + // Emulator not available or error occurred, use env vars only + if (process.env.LOG_LEVEL === 'debug') { + console.debug('Could not fetch simulator settings, using env vars only') + } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/create-bot/templates/slack-bot/src/app.ts.tmplpackages/create-bot/templates/slack-bot/src/listeners/assistant/message.tspackages/create-bot/templates/slack-bot/src/listeners/events/app-mention.tspackages/create-bot/templates/slack-bot/src/settings.ts.tmpl
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-16T23:30:02.205Z
Learnt from: tyom
Repo: tyom/botarium PR: 2
File: packages/create-bot/templates/slack-bot/src/slack/handlers.ts.tmpl:99-110
Timestamp: 2026-01-16T23:30:02.205Z
Learning: In the Slack bot template's handler (`packages/create-bot/templates/slack-bot/src/slack/handlers.ts.tmpl`), the `hasBotParticipatedInThread` function is a fallback path that only runs after bot restarts. The in-memory `threadTracker` handles active threads. The function intentionally limits to 100 messages to avoid multiple API calls and rate limit pressure for the rare edge case of threads with 100+ messages, where the failure mode is minor (bot won't auto-respond until re-mentioned).
Applied to files:
packages/create-bot/templates/slack-bot/src/app.ts.tmplpackages/create-bot/templates/slack-bot/src/listeners/assistant/message.tspackages/create-bot/templates/slack-bot/src/listeners/events/app-mention.ts
🧬 Code graph analysis (1)
packages/create-bot/templates/slack-bot/src/listeners/events/app-mention.ts (2)
packages/create-bot/templates/slack-bot/src/utils/logger.ts (1)
slackLogger(21-21)packages/create-bot/templates/slack-bot/src/utils/reactions.ts (4)
ReactionContext(19-23)addThinkingReaction(28-38)completeReactions(92-95)removeThinkingOnError(75-87)
🪛 ast-grep (0.40.5)
packages/create-bot/templates/slack-bot/src/listeners/events/app-mention.ts
[warning] 30-30: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(@?${escapeRegExp(slackConfig.app.name)}, 'gi')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 31-31: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(@?${escapeRegExp(slackConfig.app.id ?? '')}, 'gi')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🪛 Biome (2.1.2)
packages/create-bot/templates/slack-bot/src/listeners/assistant/message.ts
[error] 2-2: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 2-2: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 11-11: Expected a statement but instead found 'else'.
Expected a statement here.
(parse)
[error] 16-16: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 16-17: Illegal use of an import declaration not at the top level
move this declaration to the top level
(parse)
[error] 17-18: Illegal use of an import declaration not at the top level
move this declaration to the top level
(parse)
[error] 40-40: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 40-40: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 45-45: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 57-57: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 57-57: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 63-63: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 73-73: Expected a property, a shorthand property, a getter, a setter, or a method but instead found '{#if isAi'.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
[error] 74-75: Expected a property, a shorthand property, a getter, a setter, or a method but instead found '{'.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
[error] 75-75: Expected a property, a shorthand property, a getter, a setter, or a method but instead found '{else'.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
[error] 75-75: expected , but instead found }
Remove }
(parse)
[error] 76-76: expected , but instead found thread
Remove thread
(parse)
[error] 76-77: Expected a property, a shorthand property, a getter, a setter, or a method but instead found '{'.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
[error] 77-77: Expected a property, a shorthand property, a getter, a setter, or a method but instead found '{/if'.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
[error] 77-77: expected , but instead found }
Remove }
(parse)
[error] 78-78: expected , but instead found role
Remove role
(parse)
[error] 78-78: expected , but instead found :
Remove :
(parse)
[error] 78-78: expected , but instead found (
Remove (
(parse)
[error] 79-79: expected , but instead found :
Remove :
(parse)
[error] 80-80: expected , but instead found }
Remove }
(parse)
[error] 80-80: Expected a statement but instead found ')) ?? []'.
Expected a statement here.
(parse)
[error] 105-105: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 105-105: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 110-110: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 111-111: Expected a statement but instead found 'catch (error)'.
Expected a statement here.
(parse)
[error] 113-113: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 113-113: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 118-118: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 120-120: Expected a statement but instead found 'finally'.
Expected a statement here.
(parse)
[error] 125-125: expected } but instead the file ends
the file ends here
(parse)
[error] 20-125: Illegal use of an export declaration not at the top level
move this declaration to the top level
(parse)
[error] 125-125: expected } but instead the file ends
the file ends here
(parse)
packages/create-bot/templates/slack-bot/src/listeners/events/app-mention.ts
[error] 3-3: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 3-3: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 11-11: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 11-12: Illegal use of an import declaration not at the top level
move this declaration to the top level
(parse)
[error] 12-13: Illegal use of an import declaration not at the top level
move this declaration to the top level
(parse)
[error] 13-14: Illegal use of an import declaration not at the top level
move this declaration to the top level
(parse)
[error] 44-44: Expected a property, a shorthand property, a getter, a setter, or a method but instead found '{#if isAi'.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
[error] 44-44: expected , but instead found }
Remove }
(parse)
[error] 21-47: Illegal use of an export declaration not at the top level
move this declaration to the top level
(parse)
[error] 50-50: Expected an identifier, a member name, or a rest pattern but instead found '{#if isAi'.
Expected an identifier, a member name, or a rest pattern here.
(parse)
[error] 50-50: expected , but instead found }
Remove }
(parse)
[error] 51-51: expected , but instead found client
Remove client
(parse)
[error] 52-52: Expected an identifier, a member name, or a rest pattern but instead found '{/if'.
Expected an identifier, a member name, or a rest pattern here.
(parse)
[error] 52-52: expected , but instead found }
Remove }
(parse)
[error] 53-53: expected , but instead found say
Remove say
(parse)
[error] 58-58: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 58-58: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 65-65: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 67-67: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 67-67: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 71-71: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 93-93: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 93-93: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 96-96: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 97-97: Expected a statement but instead found 'catch (error)'.
Expected a statement here.
(parse)
[error] 99-99: Private names are only allowed on the left side of a 'in' expression
(parse)
[error] 99-99: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 102-102: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 105-105: expected } but instead the file ends
the file ends here
(parse)
🔇 Additional comments (11)
packages/create-bot/templates/slack-bot/src/app.ts.tmpl (4)
23-56: LGTM!The multi-bot race condition from the previous review has been addressed. The function now correctly captures the initial
connected_botscount and waits for an increase, ensuring this specific bot's connection is tracked rather than relying on any bot being connected.
58-84: LGTM!The
createAppfunction cleanly separates simulator and live mode configuration, with appropriate token handling for each case.
93-100: LGTM!The guard for
configServerPortat lines 97-100 correctly addresses the previous review comment. Registration now properly skips if the config server failed to start.
262-265: LGTM!Standard async entry point pattern with proper fatal error handling and exit code.
packages/create-bot/templates/slack-bot/src/listeners/events/app-mention.ts (2)
18-20: Good use of regex escaping helper.
This keeps mention-stripping robust against metacharacters.
80-89: Nice safeguard against empty responses.
This prevents Slack API rejections when the model yields no content.packages/create-bot/templates/slack-bot/src/settings.ts.tmpl (4)
1-11: LGTM!The conditional imports and simulator mode detection are correctly implemented.
12-43: LGTM!The schema correctly handles simulator mode with relaxed validation and provides sensible defaults. The optional AI configuration fields allow flexibility for different deployment scenarios.
74-101: LGTM!The settings merge logic is well-designed: simulator settings provide defaults, environment variables take precedence, and the API key exclusion when
AI_PROVIDERis explicitly set prevents credential confusion. Error handling provides clear, actionable messages.
103-113: LGTM!The use of top-level await with a mutable
letexport for hot-reloading is appropriate for ESM. Named exports maintain live bindings, so consumers will see updated values afterreloadSettings()is called.packages/create-bot/templates/slack-bot/src/listeners/assistant/message.ts (1)
29-37: Nice defensive shape check.Helps avoid processing malformed assistant payloads or blank content.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
packages/create-bot/templates/slack-bot/src/listeners/assistant/message.ts
Show resolved
Hide resolved
packages/create-bot/templates/slack-bot/src/listeners/events/app-mention.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/create-bot/templates/slack-bot/src/settings.ts.tmpl`:
- Around line 144-158: The provider selection currently instantiates models
without validating their API keys and even uses a non-null assertion for
OPENROUTER_API_KEY; add a fail-fast check that reads provider from settings
(provider variable) and validates the corresponding API key exists before
calling anthropic(modelId), google(modelId), createOpenRouter(...)(modelId) or
the OpenAI branch, throwing a clear Error when the required key (OPENAI_API_KEY,
ANTHROPIC_API_KEY, GOOGLE_API_KEY or OPENROUTER_API_KEY) is missing; remove the
non-null assertion on OPENROUTER_API_KEY and ensure createOpenRouter is only
called with a defined apiKey.
♻️ Duplicate comments (2)
packages/create-bot/src/index.ts (1)
68-68: Quote thecdpath to handle spaces.
If the target folder has spaces, the suggested command will fail. Consider wrapping the path in quotes.♻️ Proposed tweak
- printStep(1, `cd ${relativePath}`) + printStep(1, `cd "${relativePath}"`)packages/create-bot/templates/slack-bot/src/settings.ts.tmpl (1)
135-139: Update Google/OpenRouter default model IDs (still outdated).This is the same issue flagged earlier:
gemini-3.0-flash-previewandanthropic/claude-4.5-sonnetdo not match current provider IDs.What are the current model IDs (January 2026) for Google Gemini 3 Flash Preview and OpenRouter Claude Sonnet 4.5?
🧹 Nitpick comments (1)
packages/create-bot/templates/slack-bot/src/settings.ts.tmpl (1)
61-66: Add a timeout to simulator settings fetch to avoid startup hangs.If the emulator accepts a TCP connection but never replies,
fetchcan hang indefinitely. A short timeout keeps startup predictable.♻️ Proposed tweak
- try { - const response = await fetch(`${baseUrl}/api/simulator/settings`) + try { + const controller = new AbortController() + const timeoutId = setTimeout(() => controller.abort(), 3000) + const response = await fetch(`${baseUrl}/api/simulator/settings`, { + signal: controller.signal, + }) + clearTimeout(timeoutId) if (response.ok) { const data = await response.json() as { ok: boolean; settings?: Record<string, string> } return data.settings ?? {} } } catch {
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/create-bot/src/index.tspackages/create-bot/templates/slack-bot/src/settings.ts.tmpl
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-16T19:57:22.904Z
Learnt from: tyom
Repo: tyom/botarium PR: 2
File: packages/create-bot/templates/slack-bot/src/db/index.ts.tmpl:12-23
Timestamp: 2026-01-16T19:57:22.904Z
Learning: In the create-botarium CLI scaffolding (packages/create-bot), the db/ directory is skipped entirely when dbAdapter === 'none' (see scaffold.ts shouldSkipDirectory). The isDb flag is true only when dbAdapter !== 'none', and when isDb is true, exactly one of isSqlite or isPostgres will be true based on the dbAdapter value. Therefore, template files in db/ are never rendered when no database is selected, and always have exactly one database adapter flag set when they are rendered.
Applied to files:
packages/create-bot/src/index.ts
🧬 Code graph analysis (1)
packages/create-bot/src/index.ts (5)
packages/create-bot/src/utils/cli-output.ts (8)
printHeader(7-11)printStatus(13-16)printSuccess(22-26)printBlankLine(44-46)printStep(28-30)printCommand(32-34)printSectionHeader(40-42)printEnvVar(36-38)packages/create-bot/src/prompts.ts (2)
UserSelections(18-24)UserSelections(37-44)packages/create-bot/src/scaffold.ts (2)
scaffold(223-256)ScaffoldOptions(27-35)packages/create-bot/src/utils/env-vars.ts (1)
getRequiredEnvVars(36-53)packages/create-bot/src/utils/template.ts (2)
TemplateOptions(39-44)createTemplateContext(49-68)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
The HTTP server started by startConfigServer() was being used only to read the port, with the reference discarded. This caused a resource leak where the server was never closed during graceful shutdown.
conversations.replies includes the current message in its response, which was causing duplicate content when building the prompt for generateResponse. Now we extract messageTs unconditionally and filter out the message matching that timestamp before constructing history.
When slackConfig.app.name or slackConfig.app.id were empty strings, the regex became /@?/gi which incorrectly stripped all "@" characters from the message. Now the replace calls are guarded by truthy checks.
- Add fail-fast validation for required API keys before instantiating AI models, throwing clear errors when keys are missing - Remove non-null assertion on OPENROUTER_API_KEY since validation ensures it exists - Fix google model ID: gemini-3.0-flash-preview → gemini-3-flash-preview - Fix openrouter model ID: anthropic/claude-4.5-sonnet → anthropic/claude-sonnet-4.5
When a key was removed from emulator settings, the old value persisted in process.env because the code only processed keys that existed in data.settings. Now explicitly deletes previously-injected keys that are no longer present in settings.
- Replace `any` and `Function` types with `unknown` and explicit signatures - Use `stop()` instead of `close()` for Bun.serve server - Add `satisfies` pattern to fix apiKeyInfo possibly undefined - Add null guard for schema in DynamicSettings - Add provider field to PartialSelections interface
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/create-bot/src/prompts.ts (1)
84-112: ValidaterawNameso CLI and interactive flows behave consistently.When a name is supplied via flags/partials, it bypasses validation, unlike the prompt path. This can allow invalid names and inconsistent behaviour. Consider reusing the existing validator before returning selections.
🛠️ Suggested fix
// Validate required fields exist if (!rawName || !rawTemplate || !rawDatabase) { return null } + + const nameValidation = validateBotNameForPrompts(rawName) + if (nameValidation !== true) { + console.error(nameValidation) + return null + } // Validate against source of truth const template = validateOption(rawTemplate, BOT_TEMPLATES, 'template') if (!template) return null
🤖 Fix all issues with AI agents
In `@apps/electron/model-fetcher.js`:
- Around line 187-232: In categorizeOpenRouterModels, the current check for
"mini" runs before the '/o1' and '/o3' checks causing names like "o3-mini" or
"o1-mini" to be misclassified as fast; reorder the conditional checks so that
the thinking-tier detection (the lower.includes('/o1') and lower.includes('/o3')
and other thinking tokens) runs before the
"mini"/"flash"/"haiku"/"instant"/"lite" fast-tier check, ensuring models
matching '/o1' or '/o3' are pushed to tiers.thinking (then continue to call
sortOpenRouterModelsByPreference on each tier).
In `@packages/slack/src/server/state.ts`:
- Around line 1104-1109: The DEFAULT_MODELS map contains an invalid Anthropic
model id ('claude-sonnet-4-5'); update DEFAULT_MODELS (the constant named
DEFAULT_MODELS in state.ts) to use a valid Anthropic identifier such as
'claude-sonnet-4-20250514' or the alias 'claude-sonnet-4-0' so API calls to
Anthropic (e.g., code paths that read DEFAULT_MODELS['anthropic']) will use a
supported model.
🧹 Nitpick comments (6)
packages/create-bot/src/index.ts (1)
64-80: Consider quoting the path in thecdinstruction for paths with spaces.If
relativePathcontains spaces (e.g.,my project), the printed instructioncd my projectwould be incorrect. Quoting the path ensures the output is always copy-pasteable.🔧 Suggested improvement
- printStep(1, `cd ${relativePath}`) + printStep(1, `cd "${relativePath}"`)packages/slack/src/server/state.ts (2)
1120-1131: Consider providing defaults for MODEL_FAST and MODEL_THINKING.When these models are incompatible with the provider, they're deleted without fallback defaults (unlike
MODEL_DEFAULT). If downstream code expects these settings to exist after merging, this could cause issues. If they're truly optional, the current behaviour is fine.
745-774: Consider removing the unused_connectionIdparameter.The parameter is accepted but never used in the method body. Since there is only one call site in
socket-mode.ts(line 161), removing it would be a low-risk refactor. Whilst the underscore prefix adequately signals the intentionally unused parameter, removing it would further improve clarity.apps/ui/src/components/DynamicSettings.svelte (1)
556-595: Validation status may be misleading when API key is valid but model tier fetch fails.When the API key validation succeeds (line 558-559), but the subsequent
getModelTierscall fails (line 575-580), the validation status is set to 'invalid' with "Failed to fetch model tiers". This is misleading because the API key itself is valid—only the tier refresh failed.Consider keeping the 'valid' status whilst showing a separate warning for the tier fetch failure, or use a distinct status/error message that clarifies the key is valid.
Suggested approach
.catch(() => { - apiKeyValidation[key] = { - status: 'invalid', - error: 'Failed to fetch model tiers', - validatedValue: undefined, - } + // Key is valid, but tier fetch failed - don't invalidate the key + console.warn('Failed to refresh model tiers after validation') })apps/electron/electron.js (1)
364-452: Limit provider API keys exported to bots.
The generic env conversion currently exports all provider keys, so bots receive every API key even when a singleai_provideris selected. Consider skipping the known provider key fields in the generic loop and relying on the provider-specific block to inject only the active key.♻️ Proposed change
- const NON_GLOBAL_FIELDS = new Set(['bot_name', 'bot_personality']) + const NON_GLOBAL_FIELDS = new Set(['bot_name', 'bot_personality']) + const PROVIDER_KEY_FIELDS = new Set([ + 'openai_api_key', + 'anthropic_api_key', + 'google_api_key', + 'openrouter_api_key', + ]) for (const [key, value] of Object.entries(flatSettings)) { if (key.startsWith('_')) continue // Skip internal fields like _schema if (NON_GLOBAL_FIELDS.has(key)) continue // Skip bot-specific fields + if (PROVIDER_KEY_FIELDS.has(key)) continue // Inject only selected provider key if (value === undefined || value === null || value === '') continue if (typeof value !== 'string' && typeof value !== 'number') continuepackages/create-bot/templates/slack-bot/src/settings.ts.tmpl (1)
52-71: Add a timeout to simulator settings fetch to prevent startup stalls.A hung fetch can block settings load indefinitely if the emulator is slow or unreachable.
💡 Proposed timeout guard
async function fetchSimulatorSettings(): Promise<Record<string, string>> { if (!isSimulatorMode) return {} const slackApiUrl = process.env.SLACK_API_URL if (!slackApiUrl) return {} // Extract base URL (remove /api suffix if present) const baseUrl = slackApiUrl.replace(/\/api\/?$/, '') + const controller = new AbortController() + const timeoutId = setTimeout(() => controller.abort(), 5000) try { - const response = await fetch(`${baseUrl}/api/simulator/settings`) + const response = await fetch(`${baseUrl}/api/simulator/settings`, { + signal: controller.signal, + }) if (response.ok) { const data = await response.json() as { ok: boolean; settings?: Record<string, string> } return data.settings ?? {} } } catch { // Emulator not available, use env vars only + } finally { + clearTimeout(timeoutId) } return {} }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/electron/electron.jsapps/electron/model-fetcher.jsapps/ui/src/components/AppSettings.svelteapps/ui/src/components/DynamicSettings.sveltepackages/create-bot/src/index.tspackages/create-bot/src/prompts.tspackages/create-bot/src/utils/prompt-options.tspackages/create-bot/src/utils/template.tspackages/create-bot/templates/slack-bot/src/app.ts.tmplpackages/create-bot/templates/slack-bot/src/settings.ts.tmplpackages/slack/src/server/state.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/create-bot/src/utils/prompt-options.ts
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2026-01-14T21:36:07.933Z
Learnt from: tyom
Repo: tyom/botarium PR: 1
File: apps/ui/src/components/MessagePanel.svelte:10-23
Timestamp: 2026-01-14T21:36:07.933Z
Learning: In the client-only Botarium UI (Vite + Svelte), do not add or rely on SSR-specific guards (e.g., checks for sessionStorage) in components. Since the app runs only in Electron or the browser, code that accesses browser APIs will execute in a non-SSR environment, so SSR-unsafe checks are unnecessary and can be removed for maintainability.
Applied to files:
apps/ui/src/components/DynamicSettings.svelteapps/ui/src/components/AppSettings.svelte
📚 Learning: 2026-01-16T19:57:22.904Z
Learnt from: tyom
Repo: tyom/botarium PR: 2
File: packages/create-bot/templates/slack-bot/src/db/index.ts.tmpl:12-23
Timestamp: 2026-01-16T19:57:22.904Z
Learning: In the create-botarium CLI scaffolding (packages/create-bot), the db/ directory is skipped entirely when dbAdapter === 'none' (see scaffold.ts shouldSkipDirectory). The isDb flag is true only when dbAdapter !== 'none', and when isDb is true, exactly one of isSqlite or isPostgres will be true based on the dbAdapter value. Therefore, template files in db/ are never rendered when no database is selected, and always have exactly one database adapter flag set when they are rendered.
Applied to files:
packages/create-bot/src/utils/template.tspackages/create-bot/src/prompts.tspackages/create-bot/src/index.ts
📚 Learning: 2026-01-16T23:30:02.205Z
Learnt from: tyom
Repo: tyom/botarium PR: 2
File: packages/create-bot/templates/slack-bot/src/slack/handlers.ts.tmpl:99-110
Timestamp: 2026-01-16T23:30:02.205Z
Learning: In the Slack bot template's handler (`packages/create-bot/templates/slack-bot/src/slack/handlers.ts.tmpl`), the `hasBotParticipatedInThread` function is a fallback path that only runs after bot restarts. The in-memory `threadTracker` handles active threads. The function intentionally limits to 100 messages to avoid multiple API calls and rate limit pressure for the rare edge case of threads with 100+ messages, where the failure mode is minor (bot won't auto-respond until re-mentioned).
Applied to files:
packages/create-bot/templates/slack-bot/src/app.ts.tmpl
📚 Learning: 2026-01-14T21:36:12.466Z
Learnt from: tyom
Repo: tyom/botarium PR: 1
File: apps/ui/src/components/MessagePanel.svelte:10-23
Timestamp: 2026-01-14T21:36:12.466Z
Learning: The Botarium UI is a client-only Vite + Svelte application with no server-side rendering; it runs exclusively in Electron or browser environments, so checks for SSR-unsafe browser APIs like sessionStorage are unnecessary.
Applied to files:
apps/electron/electron.js
🧬 Code graph analysis (4)
packages/slack/src/server/state.ts (4)
packages/slack/src/server/types.ts (1)
ConnectedBot(348-354)packages/slack/src/lib/logger.ts (1)
stateLogger(33-33)apps/ui/src/lib/backend-state.svelte.ts (1)
settings(284-286)apps/ui/src/lib/state.svelte.ts (1)
addConnectedBot(456-463)
apps/electron/electron.js (2)
apps/electron/model-fetcher.js (18)
data(255-255)data(299-299)data(349-349)data(396-396)data(572-572)data(590-590)data(609-609)data(626-626)getModelTiers(515-527)validateApiKey(557-639)response(241-245)response(283-288)response(331-338)response(381-385)response(565-568)response(580-586)response(598-605)response(619-622)apps/electron/scripts/compile-bots.ts (3)
BotConfig(10-14)main(53-109)parseBotsYaml(25-51)
apps/electron/model-fetcher.js (1)
apps/electron/electron.js (2)
apiKey(397-397)provider(396-396)
packages/create-bot/src/prompts.ts (3)
packages/create-bot/src/utils/template.ts (4)
BotTemplate(14-14)DbAdapter(16-16)BOT_TEMPLATES(4-4)DB_ADAPTERS(11-11)packages/create-bot/src/utils/validate.ts (1)
validateBotNameForPrompts(81-84)packages/create-bot/src/utils/prompt-options.ts (3)
getTemplateChoices(36-42)getDatabaseChoices(44-50)validateOption(68-80)
🔇 Additional comments (67)
packages/create-bot/src/index.ts (3)
1-20: LGTM!The imports are well-organised, using named imports from utility modules that cleanly separate concerns (CLI output, environment variables, prompts).
29-62: LGTM!The main function has a clear, linear flow with proper exit codes for cancellation (0) and installation failure (1). The refactor to use helper functions (
printNextSteps) improves readability.
82-108: LGTM!The data-driven approach for environment variable instructions is clean and maintainable. The separate iterations for each variable category (
templateVars,aiVars,dbVars) preserve logical grouping, and thehasAnyfallback handles the edge case gracefully.packages/create-bot/templates/slack-bot/src/app.ts.tmpl (10)
1-17: LGTM!The imports are well-organised, and the bot-specific simulator tokens (
xoxb-${config.simulator.id}andxapp-${config.simulator.id}) correctly enable multi-bot support by ensuring each bot has unique credentials.
24-57: LGTM!The multi-bot race condition is properly addressed by capturing
initialConnectedBotson the first successful health check and waiting until the count increases, ensuring this specific bot's connection is tracked rather than relying on any bot being connected.
59-85: LGTM!The
createAppfunction correctly differentiates between simulator and live modes, with appropriate token sources and API URL configuration. The simplified log level mapping (debug vs info) is reasonable for a template.
87-92: LGTM!The
RegistrationResponseinterface is well-defined for the expected response shape.
94-101: LGTM!The guard for
configServerPortis properly implemented, addressing the previous review concern. Registration now correctly skips when the config server hasn't started.
133-173: LGTM!The AI settings application logic is well-designed. It properly tracks emulator-injected variables via
_EMULATOR_INJECTED_VARSto avoid overwriting user's.envvalues, handles removal of settings no longer provided by the simulator, and correctly awaitsreloadSettings()to pick up the new values.
196-201: LGTM!The config server reference is now properly stored and its port extracted, addressing the previous review concern about the server reference being discarded.
206-241: Well-documented type assertions.The WebSocket client type handling is appropriately documented with the comment explaining why
unknownand type guards are used. The reconnection logic correctly re-registers with the simulator after verifying WebSocket tracking.
264-271: LGTM - shutdown properly closes config server.The graceful shutdown now correctly stops both the Slack app and the config server, addressing the previous review concern about leaving the port bound.
279-282: LGTM!Standard entry point pattern with proper fatal error handling and exit code.
packages/slack/src/server/state.ts (5)
58-60: LGTM!Clean initialisation of the simulator settings store.
296-314: LGTM!The fallback to
botIdon line 310 correctly addresses the previous concern aboutU_undefinedcollisions for legacy bots.
340-347: LGTM!The early return for
D_prefixed channels efficiently handles dynamic DM channels without requiring a map lookup.
607-640: LGTM!The registration logic correctly handles bots in both 'disconnected' and 'connecting' states, aligning with the
tryReconnectBotlifecycle changes. The bot ID generation withapp.idpreference and UUID fallback ensures unique identifiers.
1026-1036: LGTM!The setter and getter methods for simulator settings are clean and include appropriate debug logging.
packages/create-bot/src/utils/template.ts (2)
3-36: Centralised option sources and derived types are clean.Keeps the surface consistent and reduces drift across call sites.
57-87: Auto‑derived adapter flags look solid.The context stays concise while keeping templates expressive.
packages/create-bot/src/prompts.ts (3)
2-35: Centralised options + re‑exports are tidy.
40-79: Question building is nicely streamlined.
115-123: Overwrite prompt wording is clearer.apps/electron/model-fetcher.js (7)
17-51: LGTM!The fallback model tiers are well-organised with reasonable defaults for each provider. The cache setup with a 30-minute TTL is appropriate for this use case.
58-79: LGTM!The categorisation logic correctly handles the o1/o3 thinking models first, ensuring models like
o1-miniare placed in the thinking tier rather than the fast tier. The order of conditions is appropriate.
86-107: LGTM!The Anthropic model categorisation correctly maps opus to thinking, haiku to fast, and sonnet to default, aligning with Anthropic's model naming conventions.
114-137: LGTM!The dual placement of pro models in both default and thinking tiers is a reasonable approach, as these models support complex reasoning tasks whilst also serving as general-purpose defaults.
239-428: LGTM!All fetch functions implement consistent patterns: proper authentication headers, graceful error handling with
nullreturns, and appropriate logging. The Google API key is correctly passed via thex-goog-api-keyheader as per the earlier review feedback.
515-527: LGTM!The parallel fetching strategy is efficient, and the flexible API key lookup supporting both
${provider}_api_keyand direct provider keys provides good interoperability with different settings structures.
557-639: LGTM!The
validateApiKeyfunction now correctly uses thex-goog-api-keyheader for Google validation, matching the approach infetchGoogleModels. The OpenRouter validation appropriately uses the/api/v1/auth/keyendpoint since the models endpoint is publicly accessible.apps/ui/src/components/DynamicSettings.svelte (9)
29-57: LGTM!The props interface is well-documented with clear JSDoc comments explaining the purpose of each prop. Default values are appropriately set.
59-84: LGTM!State variables are well-structured with clear typing. The use of a
SetforaiEnvOverridesis appropriate for O(1) membership checks.
86-104: LGTM!The form data synchronisation logic correctly preserves user modifications whilst updating fields that haven't been touched. Using
$effect.preensures state is synchronised before rendering.
109-124: LGTM!The initial model tiers fetch is appropriately guarded for Electron-only execution with proper error handling.
126-341: LGTM!The config fetch logic properly addresses previous review feedback: stale fetch detection, deep-merge of model tiers per provider, and comprehensive error handling. The value merging logic correctly handles provider-specific model inheritance and environment variable overrides.
343-445: LGTM!Helper functions are well-implemented. The
getModelsForTierpriority chain (dynamic API → bot config → hardcoded fallback) is sensible, andhandleProviderChangecorrectly clears model selections before the provider change to prevent stale values.
447-536: LGTM!The inheritance and override detection logic correctly handles edge cases: model fields with provider-specific validation, environment variable overrides, and the distinction between simulator settings and bot-specific fields.
598-615: LGTM!The validation status reset logic correctly handles the case where the user edits a previously validated field by comparing the stored
validatedValueagainst the current value.
675-679: LGTM!The composite key
${key}-${formData.ai_provider}ensures fields are properly re-rendered when the provider changes, avoiding stale UI states.apps/ui/src/components/AppSettings.svelte (5)
7-30: LGTM!Clean imports and well-defined props interface for the app settings modal.
38-52: LGTM!The derived
inheritableGlobalSettingscorrectly filters out bot-specific fields that shouldn't be inherited from global settings, ensuring only simulator and bot-overridable settings are considered for inheritance.
54-73: LGTM!Using
JSON.parse(JSON.stringify(...))wrapped inuntrackis an effective approach to create plain object copies and avoid Svelte's reactive proxy complications.
75-129: LGTM!The save logic correctly handles model field validation based on provider format, explicitly clears invalid model overrides, and filters out bot-overridable settings that match global values. The use of
continueensures invalid models are handled before the general override check.
160-169: LGTM!The
DynamicSettingsintegration correctly passes all required props for the inherited/override badge functionality. UsinginheritableGlobalSettingsfor bothinitialValuesandinheritedValuesensures consistent inheritance comparison.apps/electron/electron.js (17)
19-32: Imports align with the new port/model features.
54-54: Per-bot process metadata looks clean.
192-207: Port discovery helper is clear and safe.
243-265: Manifest-based bot discovery is straightforward.
269-298: Binary config assembly reads well.
302-320: Settings load logging is useful and low-risk.
326-347: Settings save logging is tidy.
350-357: Default model map is a nice central touch.
620-649: Per-bot PORT injection is well-scoped.
707-723: Pushing settings (incl. app_settings) to the emulator makes sense.
731-738: Per-bot port allocation is clear.
802-805: Bot shutdown loop matches the new Map shape.
860-867: IPC settings load tracing is helpful.
870-899: Clearing model cache on key changes is sensible.
923-945: Model IPC endpoints are clean and cohesive.
947-1013: Bot config fetch path with retries is robust.
1215-1229: Shutdown guard prevents double-cleanup.packages/create-bot/templates/slack-bot/src/settings.ts.tmpl (6)
2-7: AI provider imports are tidy and conditional.Keeps non‑AI templates lean without extra dependencies.
18-26: Simulator defaults for Slack tokens look sensible.Good balance between local ergonomics and production validation.
29-38: AI configuration keys are cleanly centralised.This makes the environment surface clearer and easier to extend.
133-140: Default model map is clear and easy to override.Nice separation between defaults and env‑driven overrides.
143-175: Provider selection flow reads cleanly.The switch is straightforward and should be easy to extend.
103-105: Template's module configuration supports top-level await correctly.The template is properly configured with
"type": "module"in package.json, and TypeScript is set to target ESNext with module preservation, ensuring top-level await will work without runtime errors.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Reorder conditional checks in categorizeOpenRouterModels so thinking tier detection (/o1, /o3) runs before fast tier (mini), preventing models like o3-mini from being misclassified as fast.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/electron/model-fetcher.js`:
- Around line 486-493: The current fallback condition only treats tiers as empty
when both tiers.fast and tiers.default are empty, which wrongly discards results
that contain only tiers.thinking; update the check in the fetch logic that uses
tiers (the block using electronLogger.debug and FALLBACK_MODEL_TIERS) to
consider tiers.thinking as well—i.e., treat tiers as empty only if fast.length
=== 0 && default.length === 0 && thinking.length === 0 (or adjust per intended
behavior), and then use FALLBACK_MODEL_TIERS[provider] when all three tiers are
empty so thinking-tier-only responses are preserved.
- Around line 17-51: FALLBACK_MODEL_TIERS in model-fetcher.js is out of sync
with simulator-settings.ts for the OpenAI provider: update the openai.fast and
openai.thinking arrays to match simulator-settings.ts (replace 'gpt-5-mini' in
the fast tier with 'gpt-4o' and replace 'o3' in the thinking tier with 'o1');
locate the FALLBACK_MODEL_TIERS object and edit the openai keys so both files
have identical tier entries and run tests or a quick diff to confirm parity.
- Around line 403-421: The allowedPrefixes array currently includes
"meta-llama/" and "mistralai/" but categorizeOpenRouterModels(modelIds) has no
matching rules for those providers, so models like "meta-llama/llama-2-70b" or
"mistralai/mistral-large" pass the filter but are not assigned to a tier; either
remove those prefixes from allowedPrefixes or add categorization logic inside
categorizeOpenRouterModels to map meta-llama/* and mistralai/* IDs to the
appropriate tier (fast/default/thinking) — update the function
categorizeOpenRouterModels to recognize the provider prefixes and return the
expected tiered groups for those IDs or prune the prefixes in allowedPrefixes to
reflect only providers that are actually categorized.
🧹 Nitpick comments (2)
apps/electron/model-fetcher.js (2)
128-133: 'Pro' models assigned to both default and thinking tiers.Unlike other categorisation functions where tiers are mutually exclusive,
promodels are pushed to bothdefaultandthinking(lines 130-132). This creates duplicate entries that may confuse UI dropdowns or selection logic. Consider whether this dual assignment is intentional, and if so, document it more clearly; otherwise, assign to a single primary tier.If single-tier assignment is preferred
// Default models: pro else if (lower.includes('pro')) { tiers.default.push(id) - // Pro models can also be used for thinking tasks - tiers.thinking.push(id) }
240-273: Consider adding a timeout to fetch requests.The
fetchcall has no timeout, which could cause the application to hang indefinitely if the OpenAI API is unresponsive. This applies to all provider fetch functions in this file.Example using AbortController
async function fetchOpenAIModels(apiKey) { try { + const controller = new AbortController() + const timeoutId = setTimeout(() => controller.abort(), 10000) const response = await fetch('https://api.openai.com/v1/models', { headers: { Authorization: `Bearer ${apiKey}`, }, + signal: controller.signal, }) + clearTimeout(timeoutId)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/electron/model-fetcher.js
🧰 Additional context used
🧬 Code graph analysis (1)
apps/electron/model-fetcher.js (1)
apps/electron/electron.js (2)
apiKey(397-397)provider(396-396)
🔇 Additional comments (11)
apps/electron/model-fetcher.js (11)
1-16: LGTM!Clean module setup with appropriate cache duration and in-memory storage for model tiers.
53-79: LGTM!The thinking tier check correctly precedes the 'mini' check, ensuring models like
o1-miniando3-miniare categorised as thinking rather than fast. Models not matching any pattern are filtered out, which is appropriate for keeping tiers focused.
81-107: LGTM!Straightforward categorisation based on Anthropic's established naming convention (opus/sonnet/haiku).
139-179: LGTM!The preference-based sorting with
startsWithmatching correctly handles versioned model IDs and falls back to alphabetical ordering for unlisted models.
181-233: LGTM!The thinking tier check correctly precedes the fast tier check, ensuring
o1-miniando3-minimodels are categorised as thinking. The/o1and/o3patterns correctly match OpenRouter's namespaced format (e.g.,openai/o3-mini).
276-322: LGTM!Correct Anthropic API authentication with
x-api-keyandanthropic-versionheaders. The response body is appropriately truncated in error logs.
324-372: LGTM!Correctly uses
x-goog-api-keyheader per Google's security best practices, and properly strips themodels/prefix from model names.
511-528: LGTM!Efficient parallel fetching with flexible API key lookup supporting both
provider_api_keyandproviderkey formats. The concurrent mutation of the result object is safe as each promise writes to a distinct key.
530-542: LGTM!Clean cache invalidation with optional provider-specific clearing.
544-550: LGTM!Simple getter for fallback tiers. Note: returns a reference to the mutable constant; if external modification is a concern, consider returning a deep clone.
552-640: LGTM!Comprehensive API key validation with provider-specific endpoints. The Google validation correctly uses the
x-goog-api-keyheader, and OpenRouter appropriately uses the/auth/keyendpoint rather than the public/modelsendpoint. Error messages are extracted with sensible fallbacks.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…gs.ts The files had diverged for the fast and thinking tiers. Updated both to use consistent values and added missing o1 model to thinking tier.
Models from these providers were in allowedPrefixes but had no matching categorization rules, causing them to be silently dropped. Added: - Fast tier: small, tiny (for mistral-small, mistral-tiny) - Default tier: llama, mistral, mixtral
The fallback condition now checks all three tiers (fast, default, thinking) before discarding API results. Previously, thinking-only responses would have been incorrectly replaced with fallback data.
- Add SQLite-backed memory system with facts and preferences tables - Show "Enable AI Memory?" prompt only when AI is enabled - Add --ai/--no-ai CLI flags for non-interactive scaffolding - Include drizzle-orm for database operations with migration support - Integrate memory context into AI response generation
- Replace two-table schema (user_preferences, memory_facts) with single memory table using type field to distinguish facts vs preferences - Add auto-create table on startup for transparent dev experience - Add extractAndSaveMemory for automatic memory extraction from messages - Add context history limit setting (CONTEXT_HISTORY_LIMIT) - Remove separate preferences module (now handled by memory type)
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/create-bot/src/prompts.ts (1)
88-116: Prevent useAi=false with non‑'none' database.
mergeAnswerscurrently acceptsdatabase='sqlite'even whenuseAiis false (e.g., via CLI flags), which can scaffold AI memory unintentionally. Consider coercing to'none'(or rejecting) when AI is disabled.🐛 Proposed fix (coerce to 'none' when AI is off)
const database = validateOption(rawDatabase, DB_ADAPTERS, 'database') if (!database) return null + const finalDatabase = useAi ? database : 'none' return { name: rawName, template, useAi, - database, + database: finalDatabase, }packages/create-bot/bin/cli.ts (1)
6-21: Guard against conflicting--aiand--no-aiflags.If both are provided, the current logic silently prefers
--ai, which is confusing. Consider failing fast with a clear error.Suggested fix
+if (values.ai && values['no-ai']) { + console.error('Cannot use --ai and --no-ai together.') + process.exit(1) +} const useAi = values.ai === true ? true : values['no-ai'] === true ? false : undefined
🤖 Fix all issues with AI agents
In `@packages/create-bot/templates/slack-bot/src/memory/index.ts`:
- Around line 25-35: The saveMemory function currently inserts options.content
verbatim which allows whitespace-only or empty strings to be persisted; update
saveMemory to normalize the content (e.g., trim() and treat null/undefined as
empty) and guard early: if the normalized content is empty, do not call
db.insert and simply return; otherwise use the normalized content value in the
insert payload (reference: saveMemory, options.content).
In `@packages/create-bot/templates/slack-bot/src/response-handler.ts.tmpl`:
- Around line 94-103: The call to extractAndSaveMemory is fire-and-forget and
can produce unhandled promise rejections; wrap it with explicit error handling
(e.g., call extractAndSaveMemory(...).catch(err => /* log or swallow */) or
await it inside a try/catch) so any rejection is caught and logged. Update the
invocation in response-handler (the extractAndSaveMemory call that passes model,
message, history: context.history, userId: context.userId, teamId:
context.teamId, sourceChannel: context.channelId, sourceThread:
context.threadTs) to attach a .catch and log the error via the existing logger
rather than letting it propagate as an unhandled rejection.
♻️ Duplicate comments (1)
packages/create-bot/templates/slack-bot/src/settings.ts.tmpl (1)
171-177: Add non-null assertion for OpenRouter API key.TypeScript cannot narrow
settings.OPENROUTER_API_KEYto non-undefined through the runtime validation ofapiKeyInfo.key(a separate extracted property). Even though validation guarantees the key exists before reaching this case, the type remainsstring | undefinedin strict mode.Suggested fix
- case 'openrouter': - return createOpenRouter({ apiKey: settings.OPENROUTER_API_KEY })(modelId) + case 'openrouter': + return createOpenRouter({ apiKey: settings.OPENROUTER_API_KEY! })(modelId)
🧹 Nitpick comments (5)
packages/create-bot/templates/slack-bot/src/memory/index.ts (3)
37-61: Clamp and validate the retrieval limit.Line 41 forwards
limitdirectly; a very large or negative value can hurt performance or yield undefined behaviour. Please bound it to a sane range.♻️ Suggested fix
export async function getMemory( context: MemoryContext, options: GetMemoryOptions = {} ): Promise<schema.Memory[]> { - const { type, category, limit = 50 } = options + const { type, category, limit = 50 } = options + const safeLimit = Math.min(Math.max(limit, 1), 200) const conditions = [ eq(schema.memory.teamId, context.teamId), eq(schema.memory.userId, context.userId), ] @@ .from(schema.memory) .where(and(...conditions)) .orderBy(desc(schema.memory.createdAt)) - .limit(limit) + .limit(safeLimit) }
113-116: Short-message gate may miss genuine facts.Line 115 skips messages under 10 chars; short facts like “I'm 28” or “NYC” will never be extracted unless explicitly flagged. Consider a lightweight heuristic to avoid dropping these.
♻️ Suggested fix
// Skip very short messages unless it's a "remember" request const isRememberRequest = /remember|don't forget|keep in mind/i.test(message) - if (message.length < 10 && !isRememberRequest) return + const looksLikeFact = /\b(i'm|i am|my|i live|i work)\b/i.test(message) + if (message.length < 10 && !isRememberRequest && !looksLikeFact) return
125-152: Surface extraction failures for troubleshooting.Lines 150–152 silently swallow all errors, which makes it hard to diagnose model or JSON parsing failures. Consider a low‑noise debug log using your existing memory logger.
packages/create-bot/templates/slack-bot/src/db/index.ts.tmpl (1)
8-31: Avoid schema drift between SQL and schema.ts.The CREATE TABLE statement duplicates the TypeScript schema, so the two can diverge over time (e.g., defaults, column changes). Consider centralising schema ownership or generating the table from the schema via a single migration step to keep them aligned.
packages/create-bot/templates/slack-bot/src/settings.ts.tmpl (1)
58-78: Add a timeout to the simulator settings fetch.A stalled connection can hang startup in simulator mode. Consider an
AbortControllerwith a short timeout.Possible change
async function fetchSimulatorSettings(): Promise<Record<string, string>> { if (!isSimulatorMode) return {} @@ - try { - const response = await fetch(`${baseUrl}/api/simulator/settings`) + const controller = new AbortController() + const timeout = setTimeout(() => controller.abort(), 3000) + try { + const response = await fetch(`${baseUrl}/api/simulator/settings`, { + signal: controller.signal, + }) if (response.ok) { const data = await response.json() as { ok: boolean; settings?: Record<string, string> } return data.settings ?? {} } } catch { // Emulator not available, use env vars only + } finally { + clearTimeout(timeout) }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
packages/create-bot/bin/cli.tspackages/create-bot/src/prompts.tspackages/create-bot/src/utils/prompt-options.tspackages/create-bot/templates/slack-bot/.env.example.tmplpackages/create-bot/templates/slack-bot/config.yaml.tmplpackages/create-bot/templates/slack-bot/drizzle.config.ts.tmplpackages/create-bot/templates/slack-bot/package.json.tmplpackages/create-bot/templates/slack-bot/src/db/index.ts.tmplpackages/create-bot/templates/slack-bot/src/db/schema.tspackages/create-bot/templates/slack-bot/src/memory/index.tspackages/create-bot/templates/slack-bot/src/response-handler.ts.tmplpackages/create-bot/templates/slack-bot/src/settings.ts.tmpl
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/create-bot/src/utils/prompt-options.ts
- packages/create-bot/templates/slack-bot/drizzle.config.ts.tmpl
- packages/create-bot/templates/slack-bot/package.json.tmpl
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-16T19:57:22.904Z
Learnt from: tyom
Repo: tyom/botarium PR: 2
File: packages/create-bot/templates/slack-bot/src/db/index.ts.tmpl:12-23
Timestamp: 2026-01-16T19:57:22.904Z
Learning: In the create-botarium CLI scaffolding (packages/create-bot), the db/ directory is skipped entirely when dbAdapter === 'none' (see scaffold.ts shouldSkipDirectory). The isDb flag is true only when dbAdapter !== 'none', and when isDb is true, exactly one of isSqlite or isPostgres will be true based on the dbAdapter value. Therefore, template files in db/ are never rendered when no database is selected, and always have exactly one database adapter flag set when they are rendered.
Applied to files:
packages/create-bot/templates/slack-bot/src/db/schema.tspackages/create-bot/src/prompts.tspackages/create-bot/templates/slack-bot/src/db/index.ts.tmplpackages/create-bot/bin/cli.ts
📚 Learning: 2026-01-16T23:30:02.205Z
Learnt from: tyom
Repo: tyom/botarium PR: 2
File: packages/create-bot/templates/slack-bot/src/slack/handlers.ts.tmpl:99-110
Timestamp: 2026-01-16T23:30:02.205Z
Learning: In the Slack bot template's handler (`packages/create-bot/templates/slack-bot/src/slack/handlers.ts.tmpl`), the `hasBotParticipatedInThread` function is a fallback path that only runs after bot restarts. The in-memory `threadTracker` handles active threads. The function intentionally limits to 100 messages to avoid multiple API calls and rate limit pressure for the rare edge case of threads with 100+ messages, where the failure mode is minor (bot won't auto-respond until re-mentioned).
Applied to files:
packages/create-bot/templates/slack-bot/src/response-handler.ts.tmpl
🧬 Code graph analysis (7)
packages/create-bot/templates/slack-bot/.env.example.tmpl (3)
packages/create-bot/src/utils/template.ts (3)
createTemplateContext(49-68)TemplateContext(6-24)TemplateOptions(39-44)packages/create-bot/templates/slack-bot/src/slack/app.ts (1)
createSlackApp(10-35)packages/create-bot/src/scaffold.ts (1)
scaffold(40-73)
packages/create-bot/templates/slack-bot/src/db/schema.ts (4)
packages/create-bot/templates/slack-bot/src/db/types.ts (2)
Memory(7-16)MemorySource(1-5)packages/create-bot/templates/slack-bot/src/db/adapters/base.ts (1)
MemoryRow(4-16)packages/create-bot/templates/slack-bot/src/db/adapters/sqlite.ts (1)
store(59-86)packages/create-bot/templates/slack-bot/src/db/adapters/postgres.ts (2)
initialize(26-50)store(52-73)
packages/create-bot/templates/slack-bot/config.yaml.tmpl (1)
apps/ui/src/lib/backend-state.svelte.ts (1)
migrateSettings(49-96)
packages/create-bot/src/prompts.ts (3)
packages/create-bot/src/utils/template.ts (4)
BotTemplate(14-14)DbAdapter(16-16)BOT_TEMPLATES(4-4)DB_ADAPTERS(11-11)packages/create-bot/src/utils/validate.ts (1)
validateBotNameForPrompts(81-84)packages/create-bot/src/utils/prompt-options.ts (3)
getTemplateChoices(36-42)getAiMemoryChoices(52-57)validateOption(75-87)
packages/create-bot/templates/slack-bot/src/db/index.ts.tmpl (4)
packages/create-bot/templates/slack-bot/src/db/repository.ts (1)
MemoryRepository(3-11)packages/create-bot/templates/slack-bot/src/db/types.ts (1)
Memory(7-16)packages/create-bot/templates/slack-bot/src/db/adapters/base.ts (1)
MemoryRow(4-16)packages/create-bot/templates/slack-bot/src/memory/store.ts (1)
MemoryStore(16-85)
packages/create-bot/templates/slack-bot/src/memory/index.ts (3)
packages/create-bot/templates/slack-bot/src/db/schema.ts (1)
Memory(15-15)packages/create-bot/templates/slack-bot/src/db/types.ts (2)
Memory(7-16)MemorySource(1-5)packages/create-bot/templates/slack-bot/src/memory/store.ts (1)
store(21-29)
packages/create-bot/bin/cli.ts (4)
packages/create-bot/src/index.ts (2)
createBot(29-62)printNextSteps(70-117)packages/create-bot/src/utils/template.ts (2)
TemplateOptions(39-44)createTemplateContext(49-68)packages/create-bot/src/scaffold.ts (1)
ScaffoldOptions(27-35)packages/create-bot/templates/slack-bot/src/ai/agents/base.ts (1)
agent(22-73)
🪛 dotenv-linter (4.0.0)
packages/create-bot/templates/slack-bot/.env.example.tmpl
[warning] 5-5: [UnorderedKey] The SLACK_APP_TOKEN key should go before the SLACK_BOT_TOKEN key
(UnorderedKey)
[warning] 7-7: [IncorrectDelimiter] The {{#if isAi}} key has incorrect delimiter
(IncorrectDelimiter)
[warning] 7-7: [KeyWithoutValue] The {{#if isAi}} key should be with a value or have an equal sign
(KeyWithoutValue)
[warning] 7-7: [LeadingCharacter] Invalid leading character detected
(LeadingCharacter)
[warning] 7-7: [LowercaseKey] The {{#if isAi}} key should be in uppercase
(LowercaseKey)
[warning] 25-25: [IncorrectDelimiter] The {{/if}} key has incorrect delimiter
(IncorrectDelimiter)
[warning] 25-25: [KeyWithoutValue] The {{/if}} key should be with a value or have an equal sign
(KeyWithoutValue)
[warning] 25-25: [LeadingCharacter] Invalid leading character detected
(LeadingCharacter)
[warning] 25-25: [LowercaseKey] The {{/if}} key should be in uppercase
(LowercaseKey)
[warning] 26-26: [IncorrectDelimiter] The {{#if isPostgres}} key has incorrect delimiter
(IncorrectDelimiter)
[warning] 26-26: [KeyWithoutValue] The {{#if isPostgres}} key should be with a value or have an equal sign
(KeyWithoutValue)
[warning] 26-26: [LeadingCharacter] Invalid leading character detected
(LeadingCharacter)
[warning] 26-26: [LowercaseKey] The {{#if isPostgres}} key should be in uppercase
(LowercaseKey)
[warning] 26-26: [UnorderedKey] The {{#if isPostgres}} key should go before the {{/if}} key
(UnorderedKey)
[warning] 30-30: [DuplicatedKey] The {{/if}} key is duplicated
(DuplicatedKey)
[warning] 30-30: [IncorrectDelimiter] The {{/if}} key has incorrect delimiter
(IncorrectDelimiter)
[warning] 30-30: [KeyWithoutValue] The {{/if}} key should be with a value or have an equal sign
(KeyWithoutValue)
[warning] 30-30: [LeadingCharacter] Invalid leading character detected
(LeadingCharacter)
[warning] 30-30: [LowercaseKey] The {{/if}} key should be in uppercase
(LowercaseKey)
🔇 Additional comments (11)
packages/create-bot/src/prompts.ts (3)
2-35: Good centralisation of prompt sources and types.The move to BOT_TEMPLATES/DB_ADAPTERS and re-exported types keeps the prompt layer consistent with shared config.
40-83: Clearer prompt flow for template and AI memory.Using shared choice builders and the conditional AI memory prompt makes the UX more predictable.
119-124: Clearer wording for overwrite prompt.Renaming to
dirPathand reflecting that in the message improves clarity.packages/create-bot/templates/slack-bot/src/memory/index.ts (1)
63-82: Memory context assembly is tidy and readable.The separation into facts and preferences with clear section headers is straightforward and easy to consume.
packages/create-bot/templates/slack-bot/src/db/index.ts.tmpl (1)
33-35: Clean export surface.Exporting a ready-to-use db plus schema is tidy and makes consumption straightforward.
packages/create-bot/templates/slack-bot/src/db/schema.ts (1)
3-16: Unified schema looks coherent.The consolidated memory table and inferred types are clear and consistent.
packages/create-bot/bin/cli.ts (1)
30-47: Help text and callsite are aligned with the new flags.Clear examples and the
useAipass‑through look consistent.packages/create-bot/templates/slack-bot/.env.example.tmpl (1)
1-30: Template looks coherent and user‑friendly.The AI and database sections are clearly gated and documented.
packages/create-bot/templates/slack-bot/config.yaml.tmpl (1)
31-121: AI settings block and grouping look consistent.Schema metadata and group ordering are clear and should render well in the UI.
packages/create-bot/templates/slack-bot/src/response-handler.ts.tmpl (1)
14-34: Interface design is clean and extensible.The handler contract is clear and should be easy to customise.
packages/create-bot/templates/slack-bot/src/settings.ts.tmpl (1)
109-112: Top-levelawaitis properly supported in the generated template.The generated Slack bot template explicitly uses ESM (
"type": "module"in package.json) with a TypeScript target ofESNextand module mode ofPreserve, which fully supports top-level await. No build configuration overrides this. The code is safe as-is.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Normalize options.content by trimming and treating null/undefined as empty, then return early if the result is empty to prevent persisting invalid memory entries.
Add .catch() handler to log errors via memoryLogger instead of letting the fire-and-forget call produce unhandled promise rejections.
1 participant
Summary
Bot Template
Multi-Bot Support
WebSocket Fixes
UX Improvements
Code Quality
Summary by CodeRabbit
New Features
Enhanced Features
Bug Fixes
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.